On May 30 13:23, Hrvoje Ribicic wrote:
> 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.
In patch 6, this condition is broken into multiple 'if-else' branches,
and here it is a single expression, so I didn't merge them.
>
> > +
> > 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
> >
> >
--
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
