On Fri, May 30, 2014 at 6:10 PM, 'Jose A. Lopes' via ganeti-devel <
[email protected]> wrote:

> * The call to update the metadata is moved inside the
>   'RunOsScriptsVirtualized' but before 'TemporaryDisks' because this
>   call updates the instance configuration in the metadata daemon, and
>   we want this configuration to be available when the helper VM
>   starts, but we don't want it to contain information about the
>   temporary disks.
>
> * The other call to update the metadata is extracted from the 'else'
>   branch to the top-level of the function because we want to update
>   metadata independently of whether the instance has an OS.
>
> * Disk paths must be overridden when running the instance inside a
>   safe virtualized environment because, from the host perserspective,
>   a disk path that looks like
>
>
> /srv/ganeti/file-storage/<instance>/e0105543-0979-4896-95c1-d2c477fafa9b.file.disk0
>
>   should be presented to the instance as
>
>     /dev/xvda (Xen)
>     /dev/vda  (KVM)
>
>   Therefore, we must pass the correct '/dev/*' disk paths to the safe
>   virtualized environment when creating the environment file.  To
>   achieve this, extend backend and RPC for export OS to allow disk
>   paths to be overridden.
>
> * The environment variable 'OS_SCRIPT' is introduce to tell the init
>
  script that runs inside the virtualized environment which OS script
>   to run.  In this patch, the OS script to run is 'create_untrusted',
>   but in the future it can be overridden for import, export, and
>   rename.
>
> Signed-off-by: Jose A. Lopes <[email protected]>
> ---
>  lib/cmdlib/instance.py | 65
> ++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 36a7a5c..6c9ba33 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -1513,7 +1513,41 @@ class LUInstanceCreate(LogicalUnit):
>            result.Warn("Failed to run rename script for %s on node %s" %
>                        (self.op.instance_name, self.pnode.name),
> self.LogWarning)
>
> -  def UpdateInstanceOsInstallPackage(self, feedback_fn, instance):
> +  def GetOsInstallPackageEnvironment(self, instance, script):
> +    """Returns the OS scripts environment for the helper VM
> +
> +    @type instance: L{objects.Instance}
> +    @param instance: instance for which the OS scripts are run
> +
> +    @type script: string
> +    @param script: script to run (e.g.,
> +                   constants.OS_SCRIPT_CREATE_UNTRUSTED)
> +
> +    @rtype: dict of string to string
> +    @return: OS scripts environment for the helper VM
> +
> +    """
> +    env = {"OS_SCRIPT": script}
> +
> +    # We pass only the instance's disks, not the helper VM's disks.
> +    if instance.hypervisor == constants.HT_KVM:
> +      prefix = "/dev/vd"
> +    elif instance.hypervisor in [constants.HT_XEN_PVM,
> constants.HT_XEN_HVM]:
> +      prefix = "/dev/xvd"
> +    else:
> +      raise errors.OpExecError("Cannot run OS scripts in a virtualized"
> +                               " environment for hypervisor '%s'"
> +                               % instance.hypervisor)
> +
> +    num_disks = len(self.cfg.GetInstanceDisks(instance.uuid))
> +
> +    for idx, disk_label in enumerate(utils.GetDiskLabels(prefix,
> num_disks + 1,
> +                                                         start=1)):
> +      env["DISK_%d_PATH" % idx] = disk_label
> +
> +    return env
> +
> +  def UpdateInstanceOsInstallPackage(self, feedback_fn, instance,
> override_env):
>      """Updates the OS parameter 'os-install-package' for an instance.
>
>      The OS install package is an archive containing an OS definition
> @@ -1531,12 +1565,17 @@ class LUInstanceCreate(LogicalUnit):
>      @param instance: instance for which the OS parameter
>                       'os-install-package' is updated
>
> +    @type override_env: dict of string to string
> +    @param override_env: if supplied, it overrides the environment of
> +                         the export OS scripts archive
> +
>      """
>      if "os-install-package" in instance.osparams:
>        feedback_fn("Using OS install package '%s'" %
>                    instance.osparams["os-install-package"])
>      else:
> -      result = self.rpc.call_os_export(instance.primary_node, instance)
> +      result = self.rpc.call_os_export(instance.primary_node, instance,
> +                                       override_env)
>        result.Raise("Could not export OS '%s'" % instance.os)
>        instance.osparams["os-install-package"] = result.payload
>
> @@ -1567,6 +1606,14 @@ class LUInstanceCreate(LogicalUnit):
>
>      disk_size = DetermineImageSize(self, install_image,
> instance.primary_node)
>
> +    env = self.GetOsInstallPackageEnvironment(
> +      instance,
> +      constants.OS_SCRIPT_CREATE_UNTRUSTED)
> +    self.UpdateInstanceOsInstallPackage(feedback_fn, instance, env)
> +    UpdateMetadata(feedback_fn, self.rpc, instance,
> +                   osparams_private=self.op.osparams_private,
> +                   osparams_secret=self.op.osparams_secret)
> +
>      with TemporaryDisk(self,
>                         instance,
>                         [(constants.DT_PLAIN, constants.DISK_RDWR,
> disk_size)],
> @@ -1749,20 +1796,18 @@ class LUInstanceCreate(LogicalUnit):
>        elif trusted:
>          self.RunOsScripts(feedback_fn, iobj)
>        else:
> -        self.UpdateInstanceOsInstallPackage(feedback_fn, iobj)
> -        UpdateMetadata(feedback_fn, self.rpc, iobj,
> -                       osparams_private=self.op.osparams_private,
> -                       osparams_secret=self.op.osparams_secret)
>          self.RunOsScriptsVirtualized(feedback_fn, iobj)
>          # Instance is modified by 'RunOsScriptsVirtualized',
>          # therefore, it must be retrieved once again from the
>          # configuration, otherwise there will be a config object
>          # version mismatch.
>          iobj = self.cfg.GetInstanceInfo(iobj.uuid)
> -    else:
> -      UpdateMetadata(feedback_fn, self.rpc, iobj,
> -                     osparams_private=self.op.osparams_private,
> -                     osparams_secret=self.op.osparams_secret)
> +
> +    # Update instance metadata so that it can be reached from the
> +    # metadata service.
> +    UpdateMetadata(feedback_fn, self.rpc, iobj,
> +                   osparams_private=self.op.osparams_private,
> +                   osparams_secret=self.op.osparams_secret)
>
>      assert not self.owned_locks(locking.LEVEL_NODE_RES)
>
> --
> 1.9.1.423.g4596e3a
>
>
LGTM, thanks

Reply via email to