hooks: Provide variables with post-opcode values
[ganeti-local] / lib / mcpu.py
index e97c743..7ab7c18 100644 (file)
@@ -1,7 +1,7 @@
 #
 #
 
-# Copyright (C) 2006, 2007 Google Inc.
+# Copyright (C) 2006, 2007, 2011 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -38,10 +38,16 @@ from ganeti import errors
 from ganeti import rpc
 from ganeti import cmdlib
 from ganeti import locking
+from ganeti import utils
+from ganeti import compat
 
 
-class _LockAcquireTimeout(Exception):
-  """Internal exception to report timeouts on acquiring locks.
+_OP_PREFIX = "Op"
+_LU_PREFIX = "LU"
+
+
+class LockAcquireTimeout(Exception):
+  """Exception to report timeouts on acquiring locks.
 
   """
 
@@ -71,60 +77,40 @@ def _CalculateLockAttemptTimeouts():
   return result
 
 
-class _LockAttemptTimeoutStrategy(object):
+class LockAttemptTimeoutStrategy(object):
   """Class with lock acquire timeout strategy.
 
   """
   __slots__ = [
-    "_attempt",
+    "_timeouts",
     "_random_fn",
-    "_start_time",
     "_time_fn",
-    "_running_timeout",
     ]
 
   _TIMEOUT_PER_ATTEMPT = _CalculateLockAttemptTimeouts()
 
-  def __init__(self, attempt=0, _time_fn=time.time, _random_fn=random.random):
+  def __init__(self, _time_fn=time.time, _random_fn=random.random):
     """Initializes this class.
 
-    @type attempt: int
-    @param attempt: Current attempt number
     @param _time_fn: Time function for unittests
     @param _random_fn: Random number generator for unittests
 
     """
     object.__init__(self)
 
-    if attempt < 0:
-      raise ValueError("Attempt must be zero or positive")
-
-    self._attempt = attempt
+    self._timeouts = iter(self._TIMEOUT_PER_ATTEMPT)
     self._time_fn = _time_fn
     self._random_fn = _random_fn
 
-    try:
-      timeout = self._TIMEOUT_PER_ATTEMPT[attempt]
-    except IndexError:
-      # No more timeouts, do blocking acquire
-      timeout = None
-
-    self._running_timeout = locking.RunningTimeout(timeout, False,
-                                                   _time_fn=_time_fn)
-
   def NextAttempt(self):
-    """Returns the strategy for the next attempt.
-
-    """
-    return _LockAttemptTimeoutStrategy(attempt=self._attempt + 1,
-                                       _time_fn=self._time_fn,
-                                       _random_fn=self._random_fn)
-
-  def CalcRemainingTimeout(self):
-    """Returns the remaining timeout.
+    """Returns the timeout for the next attempt.
 
     """
-    timeout = self._running_timeout.Remaining()
+    try:
+      timeout = self._timeouts.next()
+    except StopIteration:
+      # No more timeouts, do blocking acquire
+      timeout = None
 
     if timeout is not None:
       # Add a small variation (-/+ 5%) to timeout. This helps in situations
@@ -136,7 +122,7 @@ class _LockAttemptTimeoutStrategy(object):
     return timeout
 
 
-class OpExecCbBase:
+class OpExecCbBase: # pylint: disable-msg=W0232
   """Base class for OpCode execution callbacks.
 
   """
@@ -153,73 +139,34 @@ class OpExecCbBase:
 
     """
 
-  def ReportLocks(self, msg):
-    """Report lock operations.
+  def CheckCancel(self):
+    """Check whether job has been cancelled.
 
     """
 
 
+def _LUNameForOpName(opname):
+  """Computes the LU name for a given OpCode name.
+
+  """
+  assert opname.startswith(_OP_PREFIX), \
+      "Invalid OpCode name, doesn't start with %s: %s" % (_OP_PREFIX, opname)
+
+  return _LU_PREFIX + opname[len(_OP_PREFIX):]
+
+
+def _ComputeDispatchTable():
+  """Computes the opcode-to-lu dispatch table.
+
+  """
+  return dict((op, getattr(cmdlib, _LUNameForOpName(op.__name__)))
+              for op in opcodes.OP_MAPPING.values()
+              if op.WITH_LU)
+
+
 class Processor(object):
   """Object which runs OpCodes"""
-  DISPATCH_TABLE = {
-    # Cluster
-    opcodes.OpPostInitCluster: cmdlib.LUPostInitCluster,
-    opcodes.OpDestroyCluster: cmdlib.LUDestroyCluster,
-    opcodes.OpQueryClusterInfo: cmdlib.LUQueryClusterInfo,
-    opcodes.OpVerifyCluster: cmdlib.LUVerifyCluster,
-    opcodes.OpQueryConfigValues: cmdlib.LUQueryConfigValues,
-    opcodes.OpRenameCluster: cmdlib.LURenameCluster,
-    opcodes.OpVerifyDisks: cmdlib.LUVerifyDisks,
-    opcodes.OpSetClusterParams: cmdlib.LUSetClusterParams,
-    opcodes.OpRedistributeConfig: cmdlib.LURedistributeConfig,
-    opcodes.OpRepairDiskSizes: cmdlib.LURepairDiskSizes,
-    # node lu
-    opcodes.OpAddNode: cmdlib.LUAddNode,
-    opcodes.OpQueryNodes: cmdlib.LUQueryNodes,
-    opcodes.OpQueryNodeVolumes: cmdlib.LUQueryNodeVolumes,
-    opcodes.OpQueryNodeStorage: cmdlib.LUQueryNodeStorage,
-    opcodes.OpModifyNodeStorage: cmdlib.LUModifyNodeStorage,
-    opcodes.OpRepairNodeStorage: cmdlib.LURepairNodeStorage,
-    opcodes.OpRemoveNode: cmdlib.LURemoveNode,
-    opcodes.OpSetNodeParams: cmdlib.LUSetNodeParams,
-    opcodes.OpPowercycleNode: cmdlib.LUPowercycleNode,
-    opcodes.OpEvacuateNode: cmdlib.LUEvacuateNode,
-    opcodes.OpMigrateNode: cmdlib.LUMigrateNode,
-    # instance lu
-    opcodes.OpCreateInstance: cmdlib.LUCreateInstance,
-    opcodes.OpReinstallInstance: cmdlib.LUReinstallInstance,
-    opcodes.OpRemoveInstance: cmdlib.LURemoveInstance,
-    opcodes.OpRenameInstance: cmdlib.LURenameInstance,
-    opcodes.OpActivateInstanceDisks: cmdlib.LUActivateInstanceDisks,
-    opcodes.OpShutdownInstance: cmdlib.LUShutdownInstance,
-    opcodes.OpStartupInstance: cmdlib.LUStartupInstance,
-    opcodes.OpRebootInstance: cmdlib.LURebootInstance,
-    opcodes.OpDeactivateInstanceDisks: cmdlib.LUDeactivateInstanceDisks,
-    opcodes.OpReplaceDisks: cmdlib.LUReplaceDisks,
-    opcodes.OpRecreateInstanceDisks: cmdlib.LURecreateInstanceDisks,
-    opcodes.OpFailoverInstance: cmdlib.LUFailoverInstance,
-    opcodes.OpMigrateInstance: cmdlib.LUMigrateInstance,
-    opcodes.OpMoveInstance: cmdlib.LUMoveInstance,
-    opcodes.OpConnectConsole: cmdlib.LUConnectConsole,
-    opcodes.OpQueryInstances: cmdlib.LUQueryInstances,
-    opcodes.OpQueryInstanceData: cmdlib.LUQueryInstanceData,
-    opcodes.OpSetInstanceParams: cmdlib.LUSetInstanceParams,
-    opcodes.OpGrowDisk: cmdlib.LUGrowDisk,
-    # os lu
-    opcodes.OpDiagnoseOS: cmdlib.LUDiagnoseOS,
-    # exports lu
-    opcodes.OpQueryExports: cmdlib.LUQueryExports,
-    opcodes.OpExportInstance: cmdlib.LUExportInstance,
-    opcodes.OpRemoveExport: cmdlib.LURemoveExport,
-    # tags lu
-    opcodes.OpGetTags: cmdlib.LUGetTags,
-    opcodes.OpSearchTags: cmdlib.LUSearchTags,
-    opcodes.OpAddTags: cmdlib.LUAddTags,
-    opcodes.OpDelTags: cmdlib.LUDelTags,
-    # test lu
-    opcodes.OpTestDelay: cmdlib.LUTestDelay,
-    opcodes.OpTestAllocator: cmdlib.LUTestAllocator,
-    }
+  DISPATCH_TABLE = _ComputeDispatchTable()
 
   def __init__(self, context, ec_id):
     """Constructor for Processor
@@ -236,60 +183,7 @@ class Processor(object):
     self.rpc = rpc.RpcRunner(context.cfg)
     self.hmclass = HooksMaster
 
-  def _ReportLocks(self, level, names, shared, timeout, acquired, result):
-    """Reports lock operations.
-
-    @type level: int
-    @param level: Lock level
-    @type names: list or string
-    @param names: Lock names
-    @type shared: bool
-    @param shared: Whether the locks should be acquired in shared mode
-    @type timeout: None or float
-    @param timeout: Timeout for acquiring the locks
-    @type acquired: bool
-    @param acquired: Whether the locks have already been acquired
-    @type result: None or set
-    @param result: Result from L{locking.GanetiLockManager.acquire}
-
-    """
-    parts = []
-
-    # Build message
-    if acquired:
-      if result is None:
-        parts.append("timeout")
-      else:
-        parts.append("acquired")
-    else:
-      parts.append("waiting")
-      if timeout is None:
-        parts.append("blocking")
-      else:
-        parts.append("timeout=%0.6fs" % timeout)
-
-    parts.append(locking.LEVEL_NAMES[level])
-
-    if names == locking.ALL_SET:
-      parts.append("ALL")
-    elif isinstance(names, basestring):
-      parts.append(names)
-    else:
-      parts.append(",".join(names))
-
-    if shared:
-      parts.append("shared")
-    else:
-      parts.append("exclusive")
-
-    msg = "/".join(parts)
-
-    logging.debug("LU locks %s", msg)
-
-    if self._cbs:
-      self._cbs.ReportLocks(msg)
-
-  def _AcquireLocks(self, level, names, shared, timeout):
+  def _AcquireLocks(self, level, names, shared, timeout, priority):
     """Acquires locks via the Ganeti lock manager.
 
     @type level: int
@@ -300,14 +194,18 @@ class Processor(object):
     @param shared: Whether the locks should be acquired in shared mode
     @type timeout: None or float
     @param timeout: Timeout for acquiring the locks
+    @raise LockAcquireTimeout: In case locks couldn't be acquired in specified
+        amount of time
 
     """
-    self._ReportLocks(level, names, shared, timeout, False, None)
+    if self._cbs:
+      self._cbs.CheckCancel()
 
     acquired = self.context.glm.acquire(level, names, shared=shared,
-                                        timeout=timeout)
+                                        timeout=timeout, priority=priority)
 
-    self._ReportLocks(level, names, shared, timeout, True, acquired)
+    if acquired is None:
+      raise LockAcquireTimeout()
 
     return acquired
 
@@ -320,7 +218,7 @@ class Processor(object):
     hm = HooksMaster(self.rpc.call_hooks_runner, lu)
     h_results = hm.RunPhase(constants.HOOKS_PHASE_PRE)
     lu.HooksCallBack(constants.HOOKS_PHASE_PRE, h_results,
-                     self._Feedback, None)
+                     self.Log, None)
 
     if getattr(lu.op, "dry_run", False):
       # in this mode, no post-hooks are run, and the config is not
@@ -331,10 +229,10 @@ class Processor(object):
       return lu.dry_run_result
 
     try:
-      result = lu.Exec(self._Feedback)
+      result = lu.Exec(self.Log)
       h_results = hm.RunPhase(constants.HOOKS_PHASE_POST)
       result = lu.HooksCallBack(constants.HOOKS_PHASE_POST, h_results,
-                                self._Feedback, result)
+                                self.Log, result)
     finally:
       # FIXME: This needs locks if not lu_class.REQ_BGL
       if write_count != self.context.cfg.write_count:
@@ -342,7 +240,7 @@ class Processor(object):
 
     return result
 
-  def _LockAndExecLU(self, lu, level, calc_timeout):
+  def _LockAndExecLU(self, lu, level, calc_timeout, priority):
     """Execute a Logical Unit, with the needed locks.
 
     This is a recursive function that starts locking the given level, and
@@ -377,11 +275,7 @@ class Processor(object):
           needed_locks = lu.needed_locks[level]
 
           acquired = self._AcquireLocks(level, needed_locks, share,
-                                        calc_timeout())
-
-          if acquired is None:
-            raise _LockAcquireTimeout()
-
+                                        calc_timeout(), priority)
         else:
           # Adding locks
           add_locks = lu.add_locks[level]
@@ -400,7 +294,7 @@ class Processor(object):
         try:
           lu.acquired_locks[level] = acquired
 
-          result = self._LockAndExecLU(lu, level + 1, calc_timeout)
+          result = self._LockAndExecLU(lu, level + 1, calc_timeout, priority)
         finally:
           if level in lu.remove_locks:
             self.context.glm.remove(level, lu.remove_locks[level])
@@ -409,67 +303,63 @@ class Processor(object):
           self.context.glm.release(level)
 
     else:
-      result = self._LockAndExecLU(lu, level + 1, calc_timeout)
+      result = self._LockAndExecLU(lu, level + 1, calc_timeout, priority)
 
     return result
 
-  def ExecOpCode(self, op, cbs):
+  def ExecOpCode(self, op, cbs, timeout=None, priority=None):
     """Execute an opcode.
 
     @type op: an OpCode instance
     @param op: the opcode to be executed
     @type cbs: L{OpExecCbBase}
     @param cbs: Runtime callbacks
+    @type timeout: float or None
+    @param timeout: Maximum time to acquire all locks, None for no timeout
+    @type priority: number or None
+    @param priority: Priority for acquiring lock(s)
+    @raise LockAcquireTimeout: In case locks couldn't be acquired in specified
+        amount of time
 
     """
     if not isinstance(op, opcodes.OpCode):
       raise errors.ProgrammerError("Non-opcode instance passed"
                                    " to ExecOpcode")
 
+    lu_class = self.DISPATCH_TABLE.get(op.__class__, None)
+    if lu_class is None:
+      raise errors.OpCodeUnknown("Unknown opcode")
+
+    if timeout is None:
+      calc_timeout = lambda: None
+    else:
+      calc_timeout = utils.RunningTimeout(timeout, False).Remaining
+
     self._cbs = cbs
     try:
-      lu_class = self.DISPATCH_TABLE.get(op.__class__, None)
-      if lu_class is None:
-        raise errors.OpCodeUnknown("Unknown opcode")
-
-      timeout_strategy = _LockAttemptTimeoutStrategy()
+      # Acquire the Big Ganeti Lock exclusively if this LU requires it,
+      # and in a shared fashion otherwise (to prevent concurrent run with
+      # an exclusive LU.
+      self._AcquireLocks(locking.LEVEL_CLUSTER, locking.BGL,
+                          not lu_class.REQ_BGL, calc_timeout(),
+                          priority)
+      try:
+        lu = lu_class(self, op, self.context, self.rpc)
+        lu.ExpandNames()
+        assert lu.needed_locks is not None, "needed_locks not set by LU"
 
-      while True:
         try:
-          acquire_timeout = timeout_strategy.CalcRemainingTimeout()
-
-          # Acquire the Big Ganeti Lock exclusively if this LU requires it,
-          # and in a shared fashion otherwise (to prevent concurrent run with
-          # an exclusive LU.
-          if self._AcquireLocks(locking.LEVEL_CLUSTER, locking.BGL,
-                                not lu_class.REQ_BGL, acquire_timeout) is None:
-            raise _LockAcquireTimeout()
-
-          try:
-            lu = lu_class(self, op, self.context, self.rpc)
-            lu.ExpandNames()
-            assert lu.needed_locks is not None, "needed_locks not set by LU"
-
-            try:
-              return self._LockAndExecLU(lu, locking.LEVEL_INSTANCE,
-                                         timeout_strategy.CalcRemainingTimeout)
-            finally:
-              if self._ec_id:
-                self.context.cfg.DropECReservations(self._ec_id)
-
-          finally:
-            self.context.glm.release(locking.LEVEL_CLUSTER)
-
-        except _LockAcquireTimeout:
-          # Timeout while waiting for lock, try again
-          pass
-
-        timeout_strategy = timeout_strategy.NextAttempt()
-
+          return self._LockAndExecLU(lu, locking.LEVEL_INSTANCE, calc_timeout,
+                                     priority)
+        finally:
+          if self._ec_id:
+            self.context.cfg.DropECReservations(self._ec_id)
+      finally:
+        self.context.glm.release(locking.LEVEL_CLUSTER)
     finally:
       self._cbs = None
 
-  def _Feedback(self, *args):
+  def Log(self, *args):
     """Forward call to feedback callback function.
 
     """
@@ -481,7 +371,7 @@ class Processor(object):
 
     """
     logging.debug("Step %d/%d %s", current, total, message)
-    self._Feedback("STEP %d/%d %s" % (current, total, message))
+    self.Log("STEP %d/%d %s" % (current, total, message))
 
   def LogWarning(self, message, *args, **kwargs):
     """Log a warning to the logs and the user.
@@ -498,9 +388,9 @@ class Processor(object):
       message = message % tuple(args)
     if message:
       logging.warning(message)
-      self._Feedback(" - WARNING: %s" % message)
+      self.Log(" - WARNING: %s" % message)
     if "hint" in kwargs:
-      self._Feedback("      Hint: %s" % kwargs["hint"])
+      self.Log("      Hint: %s" % kwargs["hint"])
 
   def LogInfo(self, message, *args):
     """Log an informational message to the logs and the user.
@@ -509,11 +399,15 @@ class Processor(object):
     if args:
       message = message % tuple(args)
     logging.info(message)
-    self._Feedback(" - INFO: %s" % message)
+    self.Log(" - INFO: %s" % message)
 
   def GetECId(self):
+    """Returns the current execution context ID.
+
+    """
     if not self._ec_id:
-      errors.ProgrammerError("Tried to use execution context id when not set")
+      raise errors.ProgrammerError("Tried to use execution context id when"
+                                   " not set")
     return self._ec_id
 
 
@@ -533,50 +427,87 @@ 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()
+      (lu_env, lu_nodes_pre, lu_nodes_post) = self.lu.BuildHooksEnv()
       if lu_env:
-        for key in lu_env:
-          env["GANETI_" + key] = lu_env[key]
+        assert not compat.any(key.upper().startswith(prefix)
+                              for key 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 == "PATH" or key.startswith("GANETI_")
+                      for key in env)
+
     return self.callfn(node_list, hpath, phase, env)
 
   def RunPhase(self, phase, nodes=None):
@@ -592,17 +523,27 @@ class HooksMaster(object):
     @raise errors.HooksAbort: on failure of one of the hooks
 
     """
-    if not self.node_list[phase] and not nodes:
+    (env, node_list_pre, node_list_post) = self._BuildEnv(phase)
+    if nodes is None:
+      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
       # 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
-    hpath = self.lu.HPATH
-    if nodes is not None:
-      results = self._RunWrapper(nodes, hpath, phase)
-    else:
-      results = self._RunWrapper(self.node_list[phase], hpath, phase)
-    errs = []
+
+    results = self._RunWrapper(nodes, self.lu.HPATH, phase, env)
     if not results:
       msg = "Communication Failure"
       if phase == constants.HOOKS_PHASE_PRE:
@@ -610,15 +551,19 @@ class HooksMaster(object):
       else:
         self.lu.LogWarning(msg)
         return results
+
+    errs = []
     for node_name in results:
       res = results[node_name]
       if res.offline:
         continue
+
       msg = res.fail_msg
       if msg:
         self.lu.LogWarning("Communication failure to node %s: %s",
                            node_name, msg)
         continue
+
       for script, hkr, output in res.payload:
         if hkr == constants.HKR_FAIL:
           if phase == constants.HOOKS_PHASE_PRE:
@@ -628,8 +573,10 @@ class HooksMaster(object):
               output = "(no output)"
             self.lu.LogWarning("On %s script %s failed, output: %s" %
                                (node_name, script, output))
+
     if errs and phase == constants.HOOKS_PHASE_PRE:
       raise errors.HooksAbort(errs)
+
     return results
 
   def RunConfigUpdate(self):
@@ -639,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)