hooks: Provide variables with post-opcode values
authorMichael Hanselmann <hansmi@google.com>
Wed, 16 Mar 2011 17:35:18 +0000 (18:35 +0100)
committerMichael Hanselmann <hansmi@google.com>
Wed, 16 Mar 2011 17:58:35 +0000 (18:58 +0100)
When a hook is called, it is provided with a number of variables
describing the status of the instance/node/etc. before the operation.
Some opcodes provide extra variables to see modified values from hooks,
but that's not a generic solution.

This patch modifies the code calling hooks to generate the environment
once before and once after an opcode has been executed. Doing so should
be safe—I did not find any LU.BuildHooksEnv modifying LU instance
attributes. The values collected after running the opcode are prefixed
with “GANETI_POST_”, as opposed to “GANETI_” for pre-execution
variables. The latter are still provided for backwards compatibility.

Environment variable examples:

gnt-instance start $instance:
GANETI_INSTANCE_STATUS=down
GANETI_POST_INSTANCE_STATUS=up

gnt-instance modify -B memory=512 $instance:
GANETI_INSTANCE_BE_memory=768
GANETI_POST_INSTANCE_BE_memory=512

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

doc/hooks.rst
lib/mcpu.py
test/ganeti.hooks_unittest.py

index 6d21882..2e80d69 100644 (file)
@@ -64,7 +64,7 @@ have been run.
 Naming
 ~~~~~~
 
-The allowed names for the scripts consist of (similar to *run-parts* )
+The allowed names for the scripts consist of (similar to *run-parts*)
 upper and lower case, digits, underscores and hyphens. In other words,
 the regexp ``^[a-zA-Z0-9_-]+$``. Also, non-executable scripts will be
 ignored.
@@ -468,8 +468,10 @@ anymore in Ganeti 2.0:
 Environment variables
 ---------------------
 
-Note that all variables listed here are actually prefixed with
-*GANETI_* in order to provide a clear namespace.
+Note that all variables listed here are actually prefixed with *GANETI_*
+in order to provide a clear namespace. In addition, post-execution
+scripts receive another set of variables, prefixed with *GANETI_POST_*,
+representing the status after the opcode executed.
 
 Common variables
 ~~~~~~~~~~~~~~~~
index 7493f99..7ab7c18 100644 (file)
@@ -427,55 +427,85 @@ class HooksMaster(object):
     self.callfn = callfn
     self.lu = lu
     self.op = lu.op
-    self.env, node_list_pre, node_list_post = self._BuildEnv()
-    self.node_list = {
-      constants.HOOKS_PHASE_PRE: node_list_pre,
-      constants.HOOKS_PHASE_POST: node_list_post,
-      }
+    self.pre_env = None
+    self.pre_nodes = None
 
-  def _BuildEnv(self):
+  def _BuildEnv(self, phase):
     """Compute the environment and the target nodes.
 
     Based on the opcode and the current node list, this builds the
     environment for the hooks and the target node list for the run.
 
     """
-    env = {
-      "PATH": "/sbin:/bin:/usr/sbin:/usr/bin",
-      "GANETI_HOOKS_VERSION": constants.HOOKS_VERSION,
-      "GANETI_OP_CODE": self.op.OP_ID,
-      "GANETI_OBJECT_TYPE": self.lu.HTYPE,
-      "GANETI_DATA_DIR": constants.DATA_DIR,
-      }
+    if phase == constants.HOOKS_PHASE_PRE:
+      prefix = "GANETI_"
+    elif phase == constants.HOOKS_PHASE_POST:
+      prefix = "GANETI_POST_"
+    else:
+      raise AssertionError("Unknown phase '%s'" % phase)
+
+    env = {}
 
     if self.lu.HPATH is not None:
       (lu_env, lu_nodes_pre, lu_nodes_post) = self.lu.BuildHooksEnv()
       if lu_env:
-        assert not compat.any(key.upper().startswith("GANETI")
+        assert not compat.any(key.upper().startswith(prefix)
                               for key in lu_env)
-        env.update(("GANETI_%s" % key, value) for (key, value) in lu_env)
+        env.update(("%s%s" % (prefix, key), value)
+                   for (key, value) in lu_env.items())
     else:
       lu_nodes_pre = lu_nodes_post = []
 
+    if phase == constants.HOOKS_PHASE_PRE:
+      assert compat.all((key.startswith("GANETI_") and
+                         not key.startswith("GANETI_POST_"))
+                        for key in env)
+
+      # Record environment for any post-phase hooks
+      self.pre_env = env
+
+    elif phase == constants.HOOKS_PHASE_POST:
+      assert compat.all(key.startswith("GANETI_POST_") for key in env)
+
+      if self.pre_env:
+        assert not compat.any(key.startswith("GANETI_POST_")
+                              for key in self.pre_env)
+        env.update(self.pre_env)
+    else:
+      raise AssertionError("Unknown phase '%s'" % phase)
+
     return env, frozenset(lu_nodes_pre), frozenset(lu_nodes_post)
 
-  def _RunWrapper(self, node_list, hpath, phase):
+  def _RunWrapper(self, node_list, hpath, phase, phase_env):
     """Simple wrapper over self.callfn.
 
     This method fixes the environment before doing the rpc call.
 
     """
-    env = self.env.copy()
-    env["GANETI_HOOKS_PHASE"] = phase
-    env["GANETI_HOOKS_PATH"] = hpath
-    if self.lu.cfg is not None:
-      env["GANETI_CLUSTER"] = self.lu.cfg.GetClusterName()
-      env["GANETI_MASTER"] = self.lu.cfg.GetMasterNode()
+    cfg = self.lu.cfg
+
+    env = {
+      "PATH": "/sbin:/bin:/usr/sbin:/usr/bin",
+      "GANETI_HOOKS_VERSION": constants.HOOKS_VERSION,
+      "GANETI_OP_CODE": self.op.OP_ID,
+      "GANETI_OBJECT_TYPE": self.lu.HTYPE,
+      "GANETI_DATA_DIR": constants.DATA_DIR,
+      "GANETI_HOOKS_PHASE": phase,
+      "GANETI_HOOKS_PATH": hpath,
+      }
+
+    if cfg is not None:
+      env["GANETI_CLUSTER"] = cfg.GetClusterName()
+      env["GANETI_MASTER"] = cfg.GetMasterNode()
+
+    if phase_env:
+      assert not (set(env) & set(phase_env)), "Environment variables conflict"
+      env.update(phase_env)
 
+    # Convert everything to strings
     env = dict([(str(key), str(val)) for key, val in env.iteritems()])
 
-    assert compat.all(key == key.upper() and
-                      (key == "PATH" or key.startswith("GANETI_"))
+    assert compat.all(key == "PATH" or key.startswith("GANETI_")
                       for key in env)
 
     return self.callfn(node_list, hpath, phase, env)
@@ -493,8 +523,19 @@ class HooksMaster(object):
     @raise errors.HooksAbort: on failure of one of the hooks
 
     """
+    (env, node_list_pre, node_list_post) = self._BuildEnv(phase)
     if nodes is None:
-      nodes = self.node_list[phase]
+      if phase == constants.HOOKS_PHASE_PRE:
+        self.pre_nodes = (node_list_pre, node_list_post)
+        nodes = node_list_pre
+      elif phase == constants.HOOKS_PHASE_POST:
+        post_nodes = (node_list_pre, node_list_post)
+        assert self.pre_nodes == post_nodes, \
+               ("Node lists returned for post-phase hook don't match pre-phase"
+                " lists (pre %s, post %s)" % (self.pre_nodes, post_nodes))
+        nodes = node_list_post
+      else:
+        raise AssertionError("Unknown phase '%s'" % phase)
 
     if not nodes:
       # empty node list, we should not attempt to run this as either
@@ -502,7 +543,7 @@ class HooksMaster(object):
       # even attempt to run, or this LU doesn't do hooks at all
       return
 
-    results = self._RunWrapper(nodes, self.lu.HPATH, phase)
+    results = self._RunWrapper(nodes, self.lu.HPATH, phase, env)
     if not results:
       msg = "Communication Failure"
       if phase == constants.HOOKS_PHASE_PRE:
@@ -545,7 +586,10 @@ class HooksMaster(object):
     top-level LI if the configuration has been updated.
 
     """
+    if self.pre_env is None:
+      raise AssertionError("Pre-phase must be run before configuration update")
+
     phase = constants.HOOKS_PHASE_POST
     hpath = constants.HOOKS_NAME_CFGUPDATE
     nodes = [self.lu.cfg.GetMasterNode()]
-    self._RunWrapper(nodes, hpath, phase)
+    self._RunWrapper(nodes, hpath, phase, self.pre_env)
index 594c321..e9a1d43 100755 (executable)
@@ -35,6 +35,7 @@ from ganeti import backend
 from ganeti import constants
 from ganeti import cmdlib
 from ganeti import rpc
+from ganeti import compat
 from ganeti.constants import HKR_SUCCESS, HKR_FAIL, HKR_SKIP
 
 from mocks import FakeConfig, FakeProc, FakeContext
@@ -191,6 +192,19 @@ class TestHooksRunner(unittest.TestCase):
                            [(self._rname(fname), HKR_SUCCESS, env_exp)])
 
 
+def FakeHooksRpcSuccess(node_list, hpath, phase, env):
+  """Fake call_hooks_runner function.
+
+  @rtype: dict of node -> L{rpc.RpcResult} with a successful script result
+  @return: script execution from all nodes
+
+  """
+  rr = rpc.RpcResult
+  return dict([(node, rr(True, [("utest", constants.HKR_SUCCESS, "ok")],
+                         node=node, call='FakeScriptOk'))
+               for node in node_list])
+
+
 class TestHooksMaster(unittest.TestCase):
   """Testing case for HooksMaster"""
 
@@ -222,19 +236,6 @@ class TestHooksMaster(unittest.TestCase):
                            node=node, call='FakeScriptFail'))
                   for node in node_list])
 
-  @staticmethod
-  def _call_script_succeed(node_list, hpath, phase, env):
-    """Fake call_hooks_runner function.
-
-    @rtype: dict of node -> L{rpc.RpcResult} with a successful script result
-    @return: script execution from all nodes
-
-    """
-    rr = rpc.RpcResult
-    return dict([(node, rr(True, [("utest", constants.HKR_SUCCESS, "ok")],
-                           node=node, call='FakeScriptOk'))
-                 for node in node_list])
-
   def setUp(self):
     self.op = opcodes.OpCode()
     self.context = FakeContext()
@@ -266,10 +267,154 @@ class TestHooksMaster(unittest.TestCase):
 
   def testScriptSucceed(self):
     """Test individual rpc failure"""
-    hm = mcpu.HooksMaster(self._call_script_succeed, self.lu)
+    hm = mcpu.HooksMaster(FakeHooksRpcSuccess, self.lu)
     for phase in (constants.HOOKS_PHASE_PRE, constants.HOOKS_PHASE_POST):
       hm.RunPhase(phase)
 
 
+class FakeEnvLU(cmdlib.LogicalUnit):
+  HPATH = "env_test_lu"
+  HTYPE = constants.HTYPE_GROUP
+
+  def __init__(self, *args):
+    cmdlib.LogicalUnit.__init__(self, *args)
+    self.hook_env = None
+
+  def BuildHooksEnv(self):
+    assert self.hook_env is not None
+
+    return self.hook_env, ["localhost"], ["localhost"]
+
+
+class TestHooksRunnerEnv(unittest.TestCase):
+  def setUp(self):
+    self._rpcs = []
+
+    self.op = opcodes.OpTestDummy(result=False, messages=[], fail=False)
+    self.lu = FakeEnvLU(FakeProc(), self.op, FakeContext(), None)
+    self.hm = mcpu.HooksMaster(self._HooksRpc, self.lu)
+
+  def _HooksRpc(self, *args):
+    self._rpcs.append(args)
+    return FakeHooksRpcSuccess(*args)
+
+  def _CheckEnv(self, env, phase, hpath):
+    self.assertTrue(env["PATH"].startswith("/sbin"))
+    self.assertEqual(env["GANETI_HOOKS_PHASE"], phase)
+    self.assertEqual(env["GANETI_HOOKS_PATH"], hpath)
+    self.assertEqual(env["GANETI_OP_CODE"], self.op.OP_ID)
+    self.assertEqual(env["GANETI_OBJECT_TYPE"], constants.HTYPE_GROUP)
+    self.assertEqual(env["GANETI_HOOKS_VERSION"], str(constants.HOOKS_VERSION))
+    self.assertEqual(env["GANETI_DATA_DIR"], constants.DATA_DIR)
+
+  def testEmptyEnv(self):
+    # Check pre-phase hook
+    self.lu.hook_env = {}
+    self.hm.RunPhase(constants.HOOKS_PHASE_PRE)
+
+    (node_list, hpath, phase, env) = self._rpcs.pop(0)
+    self.assertEqual(node_list, set(["localhost"]))
+    self.assertEqual(hpath, self.lu.HPATH)
+    self.assertEqual(phase, constants.HOOKS_PHASE_PRE)
+    self._CheckEnv(env, constants.HOOKS_PHASE_PRE, self.lu.HPATH)
+
+    # Check post-phase hook
+    self.lu.hook_env = {}
+    self.hm.RunPhase(constants.HOOKS_PHASE_POST)
+
+    (node_list, hpath, phase, env) = self._rpcs.pop(0)
+    self.assertEqual(node_list, set(["localhost"]))
+    self.assertEqual(hpath, self.lu.HPATH)
+    self.assertEqual(phase, constants.HOOKS_PHASE_POST)
+    self._CheckEnv(env, constants.HOOKS_PHASE_POST, self.lu.HPATH)
+
+    self.assertRaises(IndexError, self._rpcs.pop)
+
+  def testEnv(self):
+    # Check pre-phase hook
+    self.lu.hook_env = {
+      "FOO": "pre-foo-value",
+      }
+    self.hm.RunPhase(constants.HOOKS_PHASE_PRE)
+
+    (node_list, hpath, phase, env) = self._rpcs.pop(0)
+    self.assertEqual(node_list, set(["localhost"]))
+    self.assertEqual(hpath, self.lu.HPATH)
+    self.assertEqual(phase, constants.HOOKS_PHASE_PRE)
+    self.assertEqual(env["GANETI_FOO"], "pre-foo-value")
+    self.assertFalse(compat.any(key.startswith("GANETI_POST") for key in env))
+    self._CheckEnv(env, constants.HOOKS_PHASE_PRE, self.lu.HPATH)
+
+    # Check post-phase hook
+    self.lu.hook_env = {
+      "FOO": "post-value",
+      "BAR": 123,
+      }
+    self.hm.RunPhase(constants.HOOKS_PHASE_POST)
+
+    (node_list, hpath, phase, env) = self._rpcs.pop(0)
+    self.assertEqual(node_list, set(["localhost"]))
+    self.assertEqual(hpath, self.lu.HPATH)
+    self.assertEqual(phase, constants.HOOKS_PHASE_POST)
+    self.assertEqual(env["GANETI_FOO"], "pre-foo-value")
+    self.assertEqual(env["GANETI_POST_FOO"], "post-value")
+    self.assertEqual(env["GANETI_POST_BAR"], "123")
+    self.assertFalse("GANETI_BAR" in env)
+    self._CheckEnv(env, constants.HOOKS_PHASE_POST, self.lu.HPATH)
+
+    self.assertRaises(IndexError, self._rpcs.pop)
+
+    # Check configuration update hook
+    self.hm.RunConfigUpdate()
+    (node_list, hpath, phase, env) = self._rpcs.pop(0)
+    self.assertEqual(set(node_list), set([self.lu.cfg.GetMasterNode()]))
+    self.assertEqual(hpath, constants.HOOKS_NAME_CFGUPDATE)
+    self.assertEqual(phase, constants.HOOKS_PHASE_POST)
+    self._CheckEnv(env, constants.HOOKS_PHASE_POST,
+                   constants.HOOKS_NAME_CFGUPDATE)
+    self.assertFalse(compat.any(key.startswith("GANETI_POST") for key in env))
+    self.assertEqual(env["GANETI_FOO"], "pre-foo-value")
+    self.assertRaises(IndexError, self._rpcs.pop)
+
+  def testConflict(self):
+    for name in ["DATA_DIR", "OP_CODE"]:
+      self.lu.hook_env = { name: "value" }
+      for phase in [constants.HOOKS_PHASE_PRE, constants.HOOKS_PHASE_POST]:
+        # Test using a clean HooksMaster instance
+        self.assertRaises(AssertionError,
+                          mcpu.HooksMaster(self._HooksRpc, self.lu).RunPhase,
+                          phase)
+        self.assertRaises(IndexError, self._rpcs.pop)
+
+  def testNoNodes(self):
+    self.lu.hook_env = {}
+    self.hm.RunPhase(constants.HOOKS_PHASE_PRE, nodes=[])
+    self.assertRaises(IndexError, self._rpcs.pop)
+
+  def testSpecificNodes(self):
+    self.lu.hook_env = {}
+
+    nodes = [
+      "node1.example.com",
+      "node93782.example.net",
+      ]
+
+    for phase in [constants.HOOKS_PHASE_PRE, constants.HOOKS_PHASE_POST]:
+      self.hm.RunPhase(phase, nodes=nodes)
+
+      (node_list, hpath, rpc_phase, env) = self._rpcs.pop(0)
+      self.assertEqual(set(node_list), set(nodes))
+      self.assertEqual(hpath, self.lu.HPATH)
+      self.assertEqual(rpc_phase, phase)
+      self._CheckEnv(env, phase, self.lu.HPATH)
+
+      self.assertRaises(IndexError, self._rpcs.pop)
+
+  def testRunConfigUpdateNoPre(self):
+    self.lu.hook_env = {}
+    self.assertRaises(AssertionError, self.hm.RunConfigUpdate)
+    self.assertRaises(IndexError, self._rpcs.pop)
+
+
 if __name__ == '__main__':
   testutils.GanetiTestProgram()