From: Iustin Pop <[email protected]>
Currently, when making an instance shutdown call, the decision on
whether to actually do the shutdown or simply returning is made in
backend.py, by checking whether the instance is running correctly.
However, for some hypervisors, the running/stopped state is not a
boolean on/off state but might involve an intermediate step where some
resources are allocated/used on the node. Examples could be:
- mounted directory for chroot hypervisor
- KVM runtime files
- bridge setup if not cleanup up by the hypervisor
- etc.
Currently what happens is that backend sees the instance down, and thus
doesn't call the hypervisor method anymore, thus leaving the node in an
unclean state.
In order to fix this, we need to change the semantics of shutdown as
such:
- backend doesn't do any checks anymore, but simply calls into the
appropriate hypervisor's StopInstance method
- all hypervisors must accept this call whatever the state of the
instance, and do the appropriate work based on the instance/node state
- success of shutdown is no longer signified by instance not being in
the list of running instances, but by the success (no exceptions
raised) of the StopInstance call
- retry logic in backend is simplified to remove the is running checks
The patch:
- changes backend
- changes Xen and fake hypervisors to accept not-running instances
- changes chroot hypervisor to run the unmount steps even if the
instance wasn't running before
- fixes an unrelated bug in chroot related to GetInstanceInfo
---
Note: I tested this only briefly. Will do more tests (burnin, etc.) before
committing.
lib/backend.py | 21 +--------------------
lib/hypervisor/hv_base.py | 6 ++++++
lib/hypervisor/hv_chroot.py | 31 ++++++++++++++++---------------
lib/hypervisor/hv_fake.py | 3 ---
lib/hypervisor/hv_xen.py | 5 +++++
5 files changed, 28 insertions(+), 38 deletions(-)
diff --git a/lib/backend.py b/lib/backend.py
index b903d62..537724b 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1054,26 +1054,15 @@ def InstanceShutdown(instance, timeout):
hyper = hypervisor.GetHypervisor(hv_name)
iname = instance.name
- if instance.name not in hyper.ListInstances():
- logging.info("Instance %s not running, doing nothing", iname)
- return
-
class _TryShutdown:
def __init__(self):
self.tried_once = False
def __call__(self):
- if iname not in hyper.ListInstances():
- return
-
try:
hyper.StopInstance(instance, retry=self.tried_once)
+ return
except errors.HypervisorError, err:
- if iname not in hyper.ListInstances():
- # if the instance is no longer existing, consider this a
- # success and go to cleanup
- return
-
_Fail("Failed to stop instance %s: %s", iname, err)
self.tried_once = True
@@ -1089,14 +1078,6 @@ def InstanceShutdown(instance, timeout):
try:
hyper.StopInstance(instance, force=True)
except errors.HypervisorError, err:
- if iname in hyper.ListInstances():
- # only raise an error if the instance still exists, otherwise
- # the error could simply be "instance ... unknown"!
- _Fail("Failed to force stop instance %s: %s", iname, err)
-
- time.sleep(1)
-
- if iname in hyper.ListInstances():
_Fail("Could not shutdown instance %s even by destroy", iname)
_RemoveBlockDevLinks(iname, instance.disks)
diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py
index 96e688f..cd589ba 100644
--- a/lib/hypervisor/hv_base.py
+++ b/lib/hypervisor/hv_base.py
@@ -120,6 +120,12 @@ class BaseHypervisor(object):
def StopInstance(self, instance, force=False, retry=False, name=None):
"""Stop an instance
+ If the instance is not running, the method should not raise an
+ error, but return silently. The method should also do any other
+ cleanup required such that the node environment and instance's
+ disks are left in a clean state (if this applies to the hypervisor
+ type).
+
@type instance: L{objects.Instance}
@param instance: instance to stop
@type force: boolean
diff --git a/lib/hypervisor/hv_chroot.py b/lib/hypervisor/hv_chroot.py
index 36b37a8..06832a8 100644
--- a/lib/hypervisor/hv_chroot.py
+++ b/lib/hypervisor/hv_chroot.py
@@ -130,7 +130,7 @@ class ChrootManager(hv_base.BaseHypervisor):
"""
dir_name = self._InstanceDir(instance_name)
if not self._IsDirLive(dir_name):
- raise HypervisorError("Instance %s is not running" % instance_name)
+ return None
return (instance_name, 0, 0, 0, 0, 0)
def GetAllInstancesInfo(self):
@@ -190,22 +190,23 @@ class ChrootManager(hv_base.BaseHypervisor):
name = instance.name
root_dir = self._InstanceDir(name)
- if not os.path.exists(root_dir) or not self._IsDirLive(root_dir):
+ if not os.path.exists(root_dir):
return
- # Run the chroot stop script only once
- if not retry and not force:
- result = utils.RunCmd(["chroot", root_dir, "/ganeti-chroot", "stop"])
- if result.failed:
- raise HypervisorError("Can't run the chroot stop script: %s" %
- result.output)
-
- if not force:
- utils.RunCmd(["fuser", "-k", "-TERM", "-m", root_dir])
- else:
- utils.RunCmd(["fuser", "-k", "-KILL", "-m", root_dir])
- # 2 seconds at most should be enough for KILL to take action
- time.sleep(2)
+ if self._IsDirLive(root_dir):
+ # Run the chroot stop script only once
+ if not retry and not force:
+ result = utils.RunCmd(["chroot", root_dir, "/ganeti-chroot", "stop"])
+ if result.failed:
+ raise HypervisorError("Can't run the chroot stop script: %s" %
+ result.output)
+
+ if not force:
+ utils.RunCmd(["fuser", "-k", "-TERM", "-m", root_dir])
+ else:
+ utils.RunCmd(["fuser", "-k", "-KILL", "-m", root_dir])
+ # 2 seconds at most should be enough for KILL to take action
+ time.sleep(2)
if self._IsDirLive(root_dir):
if force:
diff --git a/lib/hypervisor/hv_fake.py b/lib/hypervisor/hv_fake.py
index a97bebe..43b5ec4 100644
--- a/lib/hypervisor/hv_fake.py
+++ b/lib/hypervisor/hv_fake.py
@@ -173,9 +173,6 @@ class FakeHypervisor(hv_base.BaseHypervisor):
"""
if name is None:
name = instance.name
- if not self._IsAlive(name):
- raise errors.HypervisorError("Failed to stop instance %s: %s" %
- (name, "not running"))
self._MarkDown(name)
def RebootInstance(self, instance):
diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index a281e6a..4164719 100644
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -197,6 +197,11 @@ class XenHypervisor(hv_base.BaseHypervisor):
if name is None:
name = instance.name
self._RemoveConfigFile(name)
+
+ if self.GetInstanceInfo(name) is None:
+ # not running, we can return directly
+ return
+
if force:
command = ["xm", "destroy", name]
else:
--
1.7.0.4
--
Subscription settings:
http://groups.google.com/group/ganeti-devel/subscribe?hl=en
