Commit 07e0896f authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

Split BuildHooksEnv of LUs

Commit dd7f6776 added another call to BuildHooksEnv to provide
post-phase status variables. Since BuildHooksEnv also built the node
lists, that meant they have to be built twice. First a rather strict
check was used, but it turned out to be more tricky. Commit b423c513


had to remove the strict check again.

With this patch the function is split in two parts, one generating the
actual environment variables, and another part returning the node lists.
The former is called twice.

Unittests are updated.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
parent 959b6fe5
...@@ -83,6 +83,7 @@ class LogicalUnit(object): ...@@ -83,6 +83,7 @@ class LogicalUnit(object):
- implement CheckPrereq (except when tasklets are used) - implement CheckPrereq (except when tasklets are used)
- implement Exec (except when tasklets are used) - implement Exec (except when tasklets are used)
- implement BuildHooksEnv - implement BuildHooksEnv
- implement BuildHooksNodes
- redefine HPATH and HTYPE - redefine HPATH and HTYPE
- optionally redefine their run requirements: - optionally redefine their run requirements:
REQ_BGL: the LU needs to hold the Big Ganeti Lock exclusively REQ_BGL: the LU needs to hold the Big Ganeti Lock exclusively
...@@ -273,21 +274,28 @@ class LogicalUnit(object): ...@@ -273,21 +274,28 @@ class LogicalUnit(object):
def BuildHooksEnv(self): def BuildHooksEnv(self):
"""Build hooks environment for this LU. """Build hooks environment for this LU.
This method should return a three-node tuple consisting of: a dict @rtype: dict
containing the environment that will be used for running the @return: Dictionary containing the environment that will be used for
specific hook for this LU, a list of node names on which the hook running the hooks for this LU. The keys of the dict must not be prefixed
should run before the execution, and a list of node names on which with "GANETI_"--that'll be added by the hooks runner. The hooks runner
the hook should run after the execution. will extend the environment with additional variables. If no environment
should be defined, an empty dictionary should be returned (not C{None}).
@note: If the C{HPATH} attribute of the LU class is C{None}, this function
will not be called.
The keys of the dict must not have 'GANETI_' prefixed as this will """
be handled in the hooks runner. Also note additional keys will be raise NotImplementedError
added by the hooks runner. If the LU doesn't define any
environment, an empty dict (and not None) should be returned.
No nodes should be returned as an empty list (and not None). def BuildHooksNodes(self):
"""Build list of nodes to run LU's hooks.
Note that if the HPATH for a LU class is None, this function will @rtype: tuple; (list, list)
not be called. @return: Tuple containing a list of node names on which the hook
should run before the execution and a list of node names on which the
hook should run after the execution. No nodes should be returned as an
empty list (and not None).
@note: If the C{HPATH} attribute of the LU class is C{None}, this function
will not be called.
""" """
raise NotImplementedError raise NotImplementedError
...@@ -397,7 +405,13 @@ class NoHooksLU(LogicalUnit): # pylint: disable-msg=W0223 ...@@ -397,7 +405,13 @@ class NoHooksLU(LogicalUnit): # pylint: disable-msg=W0223
This just raises an error. This just raises an error.
""" """
assert False, "BuildHooksEnv called for NoHooksLUs" raise AssertionError("BuildHooksEnv called for NoHooksLUs")
def BuildHooksNodes(self):
"""Empty BuildHooksNodes for NoHooksLU.
"""
raise AssertionError("BuildHooksNodes called for NoHooksLU")
class Tasklet: class Tasklet:
...@@ -1109,9 +1123,15 @@ class LUClusterPostInit(LogicalUnit): ...@@ -1109,9 +1123,15 @@ class LUClusterPostInit(LogicalUnit):
"""Build hooks env. """Build hooks env.
""" """
env = {"OP_TARGET": self.cfg.GetClusterName()} return {
mn = self.cfg.GetMasterNode() "OP_TARGET": self.cfg.GetClusterName(),
return env, [], [mn] }
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
return ([], [self.cfg.GetMasterNode()])
def Exec(self, feedback_fn): def Exec(self, feedback_fn):
"""Nothing to do. """Nothing to do.
...@@ -1131,8 +1151,15 @@ class LUClusterDestroy(LogicalUnit): ...@@ -1131,8 +1151,15 @@ class LUClusterDestroy(LogicalUnit):
"""Build hooks env. """Build hooks env.
""" """
env = {"OP_TARGET": self.cfg.GetClusterName()} return {
return env, [], [] "OP_TARGET": self.cfg.GetClusterName(),
}
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
return ([], [])
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -2065,7 +2092,6 @@ class LUClusterVerify(LogicalUnit): ...@@ -2065,7 +2092,6 @@ class LUClusterVerify(LogicalUnit):
except errors.GenericError, err: except errors.GenericError, err:
self._ErrorIf(True, self.ECLUSTERCFG, None, msg % str(err)) self._ErrorIf(True, self.ECLUSTERCFG, None, msg % str(err))
def BuildHooksEnv(self): def BuildHooksEnv(self):
"""Build hooks env. """Build hooks env.
...@@ -2073,14 +2099,22 @@ class LUClusterVerify(LogicalUnit): ...@@ -2073,14 +2099,22 @@ class LUClusterVerify(LogicalUnit):
the output be logged in the verify output and the verification to fail. the output be logged in the verify output and the verification to fail.
""" """
all_nodes = self.cfg.GetNodeList() cfg = self.cfg
env = { env = {
"CLUSTER_TAGS": " ".join(self.cfg.GetClusterInfo().GetTags()) "CLUSTER_TAGS": " ".join(cfg.GetClusterInfo().GetTags())
} }
for node in self.cfg.GetAllNodesInfo().values():
env["NODE_TAGS_%s" % node.name] = " ".join(node.GetTags())
return env, [], all_nodes env.update(("NODE_TAGS_%s" % node.name, " ".join(node.GetTags()))
for node in cfg.GetAllNodesInfo().values())
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
return ([], self.cfg.GetNodeList())
def Exec(self, feedback_fn): def Exec(self, feedback_fn):
"""Verify integrity of cluster, performing various test on nodes. """Verify integrity of cluster, performing various test on nodes.
...@@ -2634,13 +2668,16 @@ class LUClusterRename(LogicalUnit): ...@@ -2634,13 +2668,16 @@ class LUClusterRename(LogicalUnit):
"""Build hooks env. """Build hooks env.
""" """
env = { return {
"OP_TARGET": self.cfg.GetClusterName(), "OP_TARGET": self.cfg.GetClusterName(),
"NEW_NAME": self.op.name, "NEW_NAME": self.op.name,
} }
mn = self.cfg.GetMasterNode()
all_nodes = self.cfg.GetNodeList() def BuildHooksNodes(self):
return env, [mn], all_nodes """Build hooks nodes.
"""
return ([self.cfg.GetMasterNode()], self.cfg.GetNodeList())
def CheckPrereq(self): def CheckPrereq(self):
"""Verify that the passed name is a valid one. """Verify that the passed name is a valid one.
...@@ -2734,12 +2771,17 @@ class LUClusterSetParams(LogicalUnit): ...@@ -2734,12 +2771,17 @@ class LUClusterSetParams(LogicalUnit):
"""Build hooks env. """Build hooks env.
""" """
env = { return {
"OP_TARGET": self.cfg.GetClusterName(), "OP_TARGET": self.cfg.GetClusterName(),
"NEW_VG_NAME": self.op.vg_name, "NEW_VG_NAME": self.op.vg_name,
} }
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
mn = self.cfg.GetMasterNode() mn = self.cfg.GetMasterNode()
return env, [mn], [mn] return ([mn], [mn])
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -3580,17 +3622,22 @@ class LUNodeRemove(LogicalUnit): ...@@ -3580,17 +3622,22 @@ class LUNodeRemove(LogicalUnit):
node would then be impossible to remove. node would then be impossible to remove.
""" """
env = { return {
"OP_TARGET": self.op.node_name, "OP_TARGET": self.op.node_name,
"NODE_NAME": self.op.node_name, "NODE_NAME": self.op.node_name,
} }
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
all_nodes = self.cfg.GetNodeList() all_nodes = self.cfg.GetNodeList()
try: try:
all_nodes.remove(self.op.node_name) all_nodes.remove(self.op.node_name)
except ValueError: except ValueError:
logging.warning("Node %s which is about to be removed not found" logging.warning("Node '%s', which is about to be removed, was not found"
" in the all nodes list", self.op.node_name) " in the list of all nodes", self.op.node_name)
return env, all_nodes, all_nodes return (all_nodes, all_nodes)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -4106,7 +4153,7 @@ class LUNodeAdd(LogicalUnit): ...@@ -4106,7 +4153,7 @@ class LUNodeAdd(LogicalUnit):
This will run on all nodes before, and on all nodes + the new node after. This will run on all nodes before, and on all nodes + the new node after.
""" """
env = { return {
"OP_TARGET": self.op.node_name, "OP_TARGET": self.op.node_name,
"NODE_NAME": self.op.node_name, "NODE_NAME": self.op.node_name,
"NODE_PIP": self.op.primary_ip, "NODE_PIP": self.op.primary_ip,
...@@ -4115,11 +4162,15 @@ class LUNodeAdd(LogicalUnit): ...@@ -4115,11 +4162,15 @@ class LUNodeAdd(LogicalUnit):
"VM_CAPABLE": str(self.op.vm_capable), "VM_CAPABLE": str(self.op.vm_capable),
} }
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
# Exclude added node # Exclude added node
pre_nodes = list(set(self.cfg.GetNodeList()) - set([self.op.node_name])) pre_nodes = list(set(self.cfg.GetNodeList()) - set([self.op.node_name]))
post_nodes = pre_nodes + [self.op.node_name, ] post_nodes = pre_nodes + [self.op.node_name, ]
return (env, pre_nodes, post_nodes) return (pre_nodes, post_nodes)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -4432,7 +4483,7 @@ class LUNodeSetParams(LogicalUnit): ...@@ -4432,7 +4483,7 @@ class LUNodeSetParams(LogicalUnit):
This runs on the master node. This runs on the master node.
""" """
env = { return {
"OP_TARGET": self.op.node_name, "OP_TARGET": self.op.node_name,
"MASTER_CANDIDATE": str(self.op.master_candidate), "MASTER_CANDIDATE": str(self.op.master_candidate),
"OFFLINE": str(self.op.offline), "OFFLINE": str(self.op.offline),
...@@ -4440,9 +4491,13 @@ class LUNodeSetParams(LogicalUnit): ...@@ -4440,9 +4491,13 @@ class LUNodeSetParams(LogicalUnit):
"MASTER_CAPABLE": str(self.op.master_capable), "MASTER_CAPABLE": str(self.op.master_capable),
"VM_CAPABLE": str(self.op.vm_capable), "VM_CAPABLE": str(self.op.vm_capable),
} }
nl = [self.cfg.GetMasterNode(),
self.op.node_name] def BuildHooksNodes(self):
return env, nl, nl """Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode(), self.op.node_name]
return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5136,9 +5191,17 @@ class LUInstanceStartup(LogicalUnit): ...@@ -5136,9 +5191,17 @@ class LUInstanceStartup(LogicalUnit):
env = { env = {
"FORCE": self.op.force, "FORCE": self.op.force,
} }
env.update(_BuildInstanceHookEnvByObject(self, self.instance)) env.update(_BuildInstanceHookEnvByObject(self, self.instance))
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5233,9 +5296,17 @@ class LUInstanceReboot(LogicalUnit): ...@@ -5233,9 +5296,17 @@ class LUInstanceReboot(LogicalUnit):
"REBOOT_TYPE": self.op.reboot_type, "REBOOT_TYPE": self.op.reboot_type,
"SHUTDOWN_TIMEOUT": self.op.shutdown_timeout, "SHUTDOWN_TIMEOUT": self.op.shutdown_timeout,
} }
env.update(_BuildInstanceHookEnvByObject(self, self.instance)) env.update(_BuildInstanceHookEnvByObject(self, self.instance))
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5315,8 +5386,14 @@ class LUInstanceShutdown(LogicalUnit): ...@@ -5315,8 +5386,14 @@ class LUInstanceShutdown(LogicalUnit):
""" """
env = _BuildInstanceHookEnvByObject(self, self.instance) env = _BuildInstanceHookEnvByObject(self, self.instance)
env["TIMEOUT"] = self.op.timeout env["TIMEOUT"] = self.op.timeout
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5375,9 +5452,14 @@ class LUInstanceReinstall(LogicalUnit): ...@@ -5375,9 +5452,14 @@ class LUInstanceReinstall(LogicalUnit):
This runs on master, primary and secondary nodes of the instance. This runs on master, primary and secondary nodes of the instance.
""" """
env = _BuildInstanceHookEnvByObject(self, self.instance) return _BuildInstanceHookEnvByObject(self, self.instance)
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5461,9 +5543,14 @@ class LUInstanceRecreateDisks(LogicalUnit): ...@@ -5461,9 +5543,14 @@ class LUInstanceRecreateDisks(LogicalUnit):
This runs on master, primary and secondary nodes of the instance. This runs on master, primary and secondary nodes of the instance.
""" """
env = _BuildInstanceHookEnvByObject(self, self.instance) return _BuildInstanceHookEnvByObject(self, self.instance)
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5528,8 +5615,14 @@ class LUInstanceRename(LogicalUnit): ...@@ -5528,8 +5615,14 @@ class LUInstanceRename(LogicalUnit):
""" """
env = _BuildInstanceHookEnvByObject(self, self.instance) env = _BuildInstanceHookEnvByObject(self, self.instance)
env["INSTANCE_NEW_NAME"] = self.op.new_name env["INSTANCE_NEW_NAME"] = self.op.new_name
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes) nl = [self.cfg.GetMasterNode()] + list(self.instance.all_nodes)
return env, nl, nl return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5639,9 +5732,15 @@ class LUInstanceRemove(LogicalUnit): ...@@ -5639,9 +5732,15 @@ class LUInstanceRemove(LogicalUnit):
""" """
env = _BuildInstanceHookEnvByObject(self, self.instance) env = _BuildInstanceHookEnvByObject(self, self.instance)
env["SHUTDOWN_TIMEOUT"] = self.op.shutdown_timeout env["SHUTDOWN_TIMEOUT"] = self.op.shutdown_timeout
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] nl = [self.cfg.GetMasterNode()]
nl_post = list(self.instance.all_nodes) + nl nl_post = list(self.instance.all_nodes) + nl
return env, nl, nl_post return (nl, nl_post)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5777,10 +5876,15 @@ class LUInstanceFailover(LogicalUnit): ...@@ -5777,10 +5876,15 @@ class LUInstanceFailover(LogicalUnit):
env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = "" env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = ""
env.update(_BuildInstanceHookEnvByObject(self, instance)) env.update(_BuildInstanceHookEnvByObject(self, instance))
nl = [self.cfg.GetMasterNode()] + list(instance.secondary_nodes)
nl_post = list(nl) return env
nl_post.append(source_node)
return env, nl, nl_post def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode()] + list(self.instance.secondary_nodes)
return (nl, nl + [self.instance.primary_node])
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -5994,12 +6098,12 @@ class LUInstanceMigrate(LogicalUnit): ...@@ -5994,12 +6098,12 @@ class LUInstanceMigrate(LogicalUnit):
source_node = instance.primary_node source_node = instance.primary_node
target_node = self._migrater.target_node target_node = self._migrater.target_node
env = _BuildInstanceHookEnvByObject(self, instance) env = _BuildInstanceHookEnvByObject(self, instance)
env["MIGRATE_LIVE"] = self._migrater.live
env["MIGRATE_CLEANUP"] = self.op.cleanup
env.update({ env.update({
"OLD_PRIMARY": source_node, "MIGRATE_LIVE": self._migrater.live,
"NEW_PRIMARY": target_node, "MIGRATE_CLEANUP": self.op.cleanup,
}) "OLD_PRIMARY": source_node,
"NEW_PRIMARY": target_node,
})
if instance.disk_template in constants.DTS_INT_MIRROR: if instance.disk_template in constants.DTS_INT_MIRROR:
env["OLD_SECONDARY"] = target_node env["OLD_SECONDARY"] = target_node
...@@ -6007,10 +6111,15 @@ class LUInstanceMigrate(LogicalUnit): ...@@ -6007,10 +6111,15 @@ class LUInstanceMigrate(LogicalUnit):
else: else:
env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = None env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = None
return env
def BuildHooksNodes(self):
"""Build hooks nodes.
"""
instance = self._migrater.instance
nl = [self.cfg.GetMasterNode()] + list(instance.secondary_nodes) nl = [self.cfg.GetMasterNode()] + list(instance.secondary_nodes)
nl_post = list(nl) return (nl, nl + [instance.primary_node])
nl_post.append(source_node)
return env, nl, nl_post
class LUInstanceMove(LogicalUnit): class LUInstanceMove(LogicalUnit):
...@@ -6043,9 +6152,18 @@ class LUInstanceMove(LogicalUnit): ...@@ -6043,9 +6152,18 @@ class LUInstanceMove(LogicalUnit):
"SHUTDOWN_TIMEOUT": self.op.shutdown_timeout, "SHUTDOWN_TIMEOUT": self.op.shutdown_timeout,
} }
env.update(_BuildInstanceHookEnvByObject(self, self.instance)) env.update(_BuildInstanceHookEnvByObject(self, self.instance))
nl = [self.cfg.GetMasterNode()] + [self.instance.primary_node, return env
self.op.target_node]
return env, nl, nl def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [
self.cfg.GetMasterNode(),
self.instance.primary_node,
self.op.target_node,
]
return (nl, nl)
def CheckPrereq(self): def CheckPrereq(self):
"""Check prerequisites. """Check prerequisites.
...@@ -6244,13 +6362,16 @@ class LUNodeMigrate(LogicalUnit): ...@@ -6244,13 +6362,16 @@ class LUNodeMigrate(LogicalUnit):
This runs on the master, the primary and all the secondaries. This runs on the master, the primary and all the secondaries.
""" """
env = { return {
"NODE_NAME": self.op.node_name, "NODE_NAME": self.op.node_name,
} }
nl = [self.cfg.GetMasterNode()] def BuildHooksNodes(self):
"""Build hooks nodes.
return (env, nl, nl) """
nl = [self.cfg.GetMasterNode()]
return (nl, nl)
class TLMigrateInstance(Tasklet): class TLMigrateInstance(Tasklet):
...@@ -7469,9 +7590,14 @@ class LUInstanceCreate(LogicalUnit): ...@@ -7469,9 +7590,14 @@ class LUInstanceCreate(LogicalUnit):
hypervisor_name=self.op.hypervisor, hypervisor_name=self.op.hypervisor,
)) ))
nl = ([self.cfg.GetMasterNode(), self.op.pnode] + return env
self.secondaries)
return env, nl, nl def BuildHooksNodes(self):
"""Build hooks nodes.
"""
nl = [self.cfg.GetMasterNode(), self.op.pnode] + self.secondaries
return nl, nl
def _ReadExportInfo(self): def _ReadExportInfo(self):
"""Reads the export information from disk. """Reads the export information from disk.
...@@ -8258,13 +8384,20 @@ class LUInstanceReplaceDisks(LogicalUnit): ...