On Wed, May 28, 2014 at 12:37 PM, 'Jose A. Lopes' via ganeti-devel <
[email protected]> wrote:

> Modify opcode and LU for instance shutdown to mark an instance as user
> down.  This is used by the watcher to cleanup instances that were
> shutdown by the user while retaining the instance status 'USER_down'.
>
> Signed-off-by: Jose A. Lopes <[email protected]>
> ---
>  lib/cmdlib/instance_operation.py | 52
> ++++++++++++++++++++++++++++++++++------
>  src/Ganeti/OpCodes.hs            |  1 +
>  src/Ganeti/OpParams.hs           |  7 ++++++
>  3 files changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/lib/cmdlib/instance_operation.py
> b/lib/cmdlib/instance_operation.py
> index d210b72..22e982d 100644
> --- a/lib/cmdlib/instance_operation.py
> +++ b/lib/cmdlib/instance_operation.py
> @@ -45,6 +45,15 @@ from ganeti.cmdlib.instance_utils import
> BuildInstanceHookEnvByObject, \
>  from ganeti.hypervisor import hv_base
>
>
> +def _IsInstanceUserDown(cluster, instance, instance_info):
> +  hvparams = cluster.FillHV(instance, skip_globals=True)
> +  return instance_info and \
> +      "state" in instance_info and \
> +      hv_base.HvInstanceState.IsShutdown(instance_info["state"]) and \
> +      (instance.hypervisor != constants.HT_KVM or
> +       hvparams[constants.HV_KVM_USER_SHUTDOWN])
> +
>

Patch 6 has code that is very similar - please take a look if this can be
refactored.


> +
>  class LUInstanceStartup(LogicalUnit):
>    """Starts an instance.
>
> @@ -183,6 +192,17 @@ class LUInstanceShutdown(LogicalUnit):
>    def ExpandNames(self):
>      self._ExpandAndLockInstance()
>
> +  def CheckArguments(self):
> +    """Check arguments.
> +
> +    """
> +    if self.op.no_remember and self.op.admin_state_source:
> +      self.LogWarning("Parameter 'admin_state_source' has no effect if
> used"
> +                      " with parameter 'no_remember'")
> +
> +    if not self.op.admin_state_source:
> +      self.op.admin_state_source = constants.ADMIN_SOURCE
> +
>    def BuildHooksEnv(self):
>      """Build hooks env.
>
> @@ -210,10 +230,10 @@ class LUInstanceShutdown(LogicalUnit):
>      assert self.instance is not None, \
>        "Cannot retrieve locked instance %s" % self.op.instance_name
>
> -    if not self.op.force:
> -      CheckInstanceState(self, self.instance, INSTANCE_ONLINE)
> -    else:
> +    if self.op.force:
>        self.LogWarning("Ignoring offline instance check")
> +    else:
> +      CheckInstanceState(self, self.instance, INSTANCE_ONLINE)
>
>      self.primary_offline = \
>        self.cfg.GetNodeInfo(self.instance.primary_node).offline
> @@ -223,6 +243,23 @@ class LUInstanceShutdown(LogicalUnit):
>      else:
>        CheckNodeOnline(self, self.instance.primary_node)
>
> +    if self.op.admin_state_source == constants.USER_SOURCE:
> +      cluster = self.cfg.GetClusterInfo()
> +
> +      result = self.rpc.call_instance_info(
> +        self.instance.primary_node,
> +        self.instance.name,
> +        self.instance.hypervisor,
> +        cluster.hvparams[self.instance.hypervisor])
> +      result.Raise("Error checking instance '%s'" % self.instance.name,
> +                   prereq=True)
> +
> +      if not _IsInstanceUserDown(cluster,
> +                                 self.instance,
> +                                 result.payload):
> +        raise errors.OpPrereqError("Instance '%s' was not shutdown by the
> user"
> +                                   % self.instance.name)

+
>    def Exec(self, feedback_fn):
>      """Shutdown the instance.
>
> @@ -230,7 +267,10 @@ class LUInstanceShutdown(LogicalUnit):
>      # If the instance is offline we shouldn't mark it as down, as that
>      # resets the offline flag.
>      if not self.op.no_remember and self.instance.admin_state in
> INSTANCE_ONLINE:
> -      self.cfg.MarkInstanceDown(self.instance.uuid)
> +      if self.op.admin_state_source == constants.ADMIN_SOURCE:
> +        self.cfg.MarkInstanceDown(self.instance.uuid)
> +      elif self.op.admin_state_source == constants.USER_SOURCE:
> +        self.cfg.MarkInstanceUserDown(self.instance.uuid)
>
>      if self.primary_offline:
>        assert self.op.ignore_offline_nodes
> @@ -240,9 +280,7 @@ class LUInstanceShutdown(LogicalUnit):
>          self.instance.primary_node,
>          self.instance,
>          self.op.timeout, self.op.reason)
> -      msg = result.fail_msg
> -      if msg:
> -        self.LogWarning("Could not shutdown instance: %s", msg)
> +      result.Raise("Could not shutdown instance '%s'", self.instance.name
> )
>
>        ShutdownInstanceDisks(self, self.instance)
>
> diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
> index eb24b0a..3975afe 100644
> --- a/src/Ganeti/OpCodes.hs
> +++ b/src/Ganeti/OpCodes.hs
> @@ -511,6 +511,7 @@ $(genOpCode "OpCode"
>       , pIgnoreOfflineNodes
>       , pShutdownTimeout'
>       , pNoRemember
> +     , pAdminStateSource
>       ],
>       "instance_name")
>    , ("OpInstanceReboot",
> diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs
> index c330c83..1d077d1 100644
> --- a/src/Ganeti/OpParams.hs
> +++ b/src/Ganeti/OpParams.hs
> @@ -260,6 +260,7 @@ module Ganeti.OpParams
>    , pReason
>    , pSequential
>    , pEnabledDiskTemplates
> +  , pAdminStateSource
>    ) where
>
>  import Control.Monad (liftM, mplus)
> @@ -1671,3 +1672,9 @@ pNetworkLink :: Field
>  pNetworkLink =
>    withDoc "Network link when connecting to a group" $
>    simpleField "network_link" [t| NonEmptyString |]
> +
> +pAdminStateSource :: Field
> +pAdminStateSource =
> +  withDoc "Who last changed the instance admin state" .
> +  optionalField $
> +  simpleField "admin_state_source" [t| AdminStateSource |]
> --
> 1.9.1.423.g4596e3a
>
>

Reply via email to