LGTM, thanks
On Wed, May 28, 2014 at 12:37 PM, 'Jose A. Lopes' via ganeti-devel < [email protected]> wrote: > * Given the previous changes to user down report status, we can lift > the restriction that user down instances could not be directly > started. This patch modifies instance startup to first cleanup user > down instances before starting them. > > * Also, the opcode for instance create is given a shutdown timeout and > the unit tests are updated accordingly. > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > lib/cmdlib/instance_operation.py | 21 ++++++++++++++++----- > src/Ganeti/OpCodes.hs | 2 ++ > test/hs/Test/Ganeti/OpCodes.hs | 13 ++++++++++--- > 3 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/lib/cmdlib/instance_operation.py > b/lib/cmdlib/instance_operation.py > index 22e982d..6f9258a 100644 > --- a/lib/cmdlib/instance_operation.py > +++ b/lib/cmdlib/instance_operation.py > @@ -145,12 +145,14 @@ class LUInstanceStartup(LogicalUnit): > remote_info.Raise("Error checking node %s" % > self.cfg.GetNodeName(self.instance.primary_node), > prereq=True, ecode=errors.ECODE_ENVIRON) > + > + self.requires_cleanup = False > + > if remote_info.payload: > - if > hv_base.HvInstanceState.IsShutdown(remote_info.payload["state"]): > - raise errors.OpPrereqError("Instance '%s' was shutdown by the > user," > - " please shutdown the instance > before" > - " starting it again" % > self.instance.name, > - errors.ECODE_INVAL) > + if _IsInstanceUserDown(self.cfg.GetClusterInfo(), > + self.instance, > + remote_info.payload): > + self.requires_cleanup = True > else: # not running already > CheckNodeFreeMemory( > self, self.instance.primary_node, > @@ -169,6 +171,15 @@ class LUInstanceStartup(LogicalUnit): > assert self.op.ignore_offline_nodes > self.LogInfo("Primary node offline, marked instance as started") > else: > + if self.requires_cleanup: > + result = self.rpc.call_instance_shutdown( > + self.instance.primary_node, > + self.instance, > + self.op.shutdown_timeout, self.op.reason) > + result.Raise("Could not shutdown instance '%s'", > self.instance.name) > + > + ShutdownInstanceDisks(self, self.instance) > + > StartInstanceDisks(self, self.instance, self.op.force) > > result = \ > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index 3975afe..8e7bf4f 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -500,6 +500,8 @@ $(genOpCode "OpCode" > , pTempBeParams > , pNoRemember > , pStartupPaused > + -- timeout to cleanup a user down instance > + , pShutdownTimeout > ], > "instance_name") > , ("OpInstanceShutdown", > diff --git a/test/hs/Test/Ganeti/OpCodes.hs > b/test/hs/Test/Ganeti/OpCodes.hs > index 84c60f3..f677de3 100644 > --- a/test/hs/Test/Ganeti/OpCodes.hs > +++ b/test/hs/Test/Ganeti/OpCodes.hs > @@ -256,9 +256,16 @@ instance Arbitrary OpCodes.OpCode where > OpCodes.OpInstanceRename <$> genFQDN <*> return Nothing <*> > genNodeNameNE <*> arbitrary <*> arbitrary > "OP_INSTANCE_STARTUP" -> > - OpCodes.OpInstanceStartup <$> genFQDN <*> return Nothing <*> > - arbitrary <*> arbitrary <*> pure emptyJSObject <*> > - pure emptyJSObject <*> arbitrary <*> arbitrary > + OpCodes.OpInstanceStartup <$> > + genFQDN <*> -- instance_name > + return Nothing <*> -- instance_uuid > + arbitrary <*> -- force > + arbitrary <*> -- ignore_offline_nodes > + pure emptyJSObject <*> -- hvparams > + pure emptyJSObject <*> -- beparams > + arbitrary <*> -- no_remember > + arbitrary <*> -- startup_paused > + arbitrary -- shutdown_timeout > "OP_INSTANCE_SHUTDOWN" -> > OpCodes.OpInstanceShutdown <$> genFQDN <*> return Nothing <*> > arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> > arbitrary > -- > 1.9.1.423.g4596e3a > >
