Hi Ilias,
Can you please explain me why this change is necessary? I don't
really see why moving the disks to top-level requires changing the
hooks. Your comment says that the instance is not available in the
config anymore, but I guess that was also true before this change.
Thanks,
Jose
On Apr 14 14:29, Ilias Tsitsimpis wrote:
> The function BuildHooksEnv in LUInstanceRemove, calls the function
> BuildInstanceHookEnvByObject which needs to query the config to get
> the secondary_nodes/disks of an instance. The function BuildHooksEnv
> will be called before and after the Exec function. The second time
> it will fail to query for the secondary_nodes/disks of the instance as
> the instance will not be available at the config any more.
>
> This patch changes BuildInstanceHookEnvByObject to allow it
> to override the secondary_nodes/disks of an instance.
> ---
> lib/cmdlib/backup.py | 7 ++++++-
> lib/cmdlib/instance.py | 6 +++++-
> lib/cmdlib/instance_utils.py | 16 +++++++++++++---
> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
> index 67582c4..4787e3d 100644
> --- a/lib/cmdlib/backup.py
> +++ b/lib/cmdlib/backup.py
> @@ -173,7 +173,9 @@ class LUBackupExport(LogicalUnit):
> "REMOVE_INSTANCE": str(bool(self.op.remove_instance)),
> }
>
> - env.update(BuildInstanceHookEnvByObject(self, self.instance))
> + env.update(BuildInstanceHookEnvByObject(
> + self, self.instance,
> + secondary_nodes=self.secondary_nodes, disks=self.inst_disks))
>
> return env
>
> @@ -312,6 +314,9 @@ class LUBackupExport(LogicalUnit):
> raise errors.OpPrereqError("Zeroing timeout options can only be used"
> " only with the --zero-free-space option")
>
> + self.secondary_nodes = self.instance.secondary_nodes
> + self.inst_disks = self.instance.disks
> +
> def _CleanupExports(self, feedback_fn):
> """Removes exports of current instance from all other nodes.
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 3a7a5ac..e5e29fd 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -1794,7 +1794,9 @@ class LUInstanceRemove(LogicalUnit):
> This runs on master, primary and secondary nodes of the instance.
>
> """
> - env = BuildInstanceHookEnvByObject(self, self.instance)
> + env = BuildInstanceHookEnvByObject(self, self.instance,
> + secondary_nodes=self.secondary_nodes,
> + disks=self.inst_disks)
> env["SHUTDOWN_TIMEOUT"] = self.op.shutdown_timeout
> return env
>
> @@ -1815,6 +1817,8 @@ class LUInstanceRemove(LogicalUnit):
> self.instance = self.cfg.GetInstanceInfo(self.op.instance_uuid)
> assert self.instance is not None, \
> "Cannot retrieve locked instance %s" % self.op.instance_name
> + self.secondary_nodes = self.instance.secondary_nodes
> + self.inst_disks = self.instance.disks
>
> def Exec(self, feedback_fn):
> """Remove the instance.
> diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py
> index 4670397..628c1bb 100644
> --- a/lib/cmdlib/instance_utils.py
> +++ b/lib/cmdlib/instance_utils.py
> @@ -147,7 +147,8 @@ def BuildInstanceHookEnv(name, primary_node_name,
> secondary_node_names, os_type,
> return env
>
>
> -def BuildInstanceHookEnvByObject(lu, instance, override=None):
> +def BuildInstanceHookEnvByObject(lu, instance, secondary_nodes=None,
> + disks=None, override=None):
> """Builds instance related env variables for hooks from an object.
>
> @type lu: L{LogicalUnit}
> @@ -165,10 +166,19 @@ def BuildInstanceHookEnvByObject(lu, instance,
> override=None):
> cluster = lu.cfg.GetClusterInfo()
> bep = cluster.FillBE(instance)
> hvp = cluster.FillHV(instance)
> +
> + # Override secondary_nodes
> + if secondary_nodes is None:
> + secondary_nodes = instance.secondary_nodes
> +
> + # Override disks
> + if disks is None:
> + disks = instance.disks
> +
> args = {
> "name": instance.name,
> "primary_node_name": lu.cfg.GetNodeName(instance.primary_node),
> - "secondary_node_names": lu.cfg.GetNodeNames(instance.secondary_nodes),
> + "secondary_node_names": lu.cfg.GetNodeNames(secondary_nodes),
> "os_type": instance.os,
> "status": instance.admin_state,
> "maxmem": bep[constants.BE_MAXMEM],
> @@ -177,7 +187,7 @@ def BuildInstanceHookEnvByObject(lu, instance,
> override=None):
> "nics": NICListToTuple(lu, instance.nics),
> "disk_template": instance.disk_template,
> "disks": [(disk.name, disk.uuid, disk.size, disk.mode)
> - for disk in instance.disks],
> + for disk in disks],
> "bep": bep,
> "hvp": hvp,
> "hypervisor_name": instance.hypervisor,
> --
> 1.9.1
>
--
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
