Make volume detachment a co-routine

Change-Id: I799f891010fb75c80afd30ded761aa2b3b632394

diff --git a/heat/engine/clients.py b/heat/engine/clients.py
index feb6c9cbe06f33463a870ac36ca7209468c37c57..31f418ae2cc8c67d124f137953cbbbcd0c69a67f 100644 (file)
--- a/heat/engine/clients.py
+++ b/heat/engine/clients.py
@@ -13,7 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import eventlet
 from oslo.config import cfg
 
 from heat.openstack.common import importutils
@@ -192,41 +191,6 @@ class OpenStackClients(object):
 
         return self._cinder
 
-    def detach_volume_from_instance(self, server_id, volume_id):
-        logger.info('VolumeAttachment un-attaching %s %s' %
-                    (server_id, volume_id))
-
-        try:
-            vol = self.cinder().volumes.get(volume_id)
-        except cinderclient.exceptions.NotFound:
-            logger.warning('Volume %s - not found' %
-                          (volume_id))
-            return
-        try:
-            self.nova().volumes.delete_server_volume(server_id,
-                                                     volume_id)
-        except novaclient.exceptions.NotFound:
-            logger.warning('Deleting VolumeAttachment %s %s - not found' %
-                          (server_id, volume_id))
-        try:
-            logger.info('un-attaching %s, status %s' % (volume_id, vol.status))
-            while vol.status == 'in-use':
-                logger.info('trying to un-attach %s, but still %s' %
-                            (volume_id, vol.status))
-                eventlet.sleep(1)
-                try:
-                    self.nova().volumes.delete_server_volume(
-                        server_id,
-                        volume_id)
-                except Exception:
-                    pass
-                vol.get()
-            logger.info('volume status of %s now %s' % (volume_id, vol.status))
-        except cinderclient.exceptions.NotFound:
-            logger.warning('Volume %s - not found' %
-                          (volume_id))
-
-
 if cfg.CONF.cloud_backend:
     cloud_backend_module = importutils.import_module(cfg.CONF.cloud_backend)
     Clients = cloud_backend_module.Clients

diff --git a/heat/engine/resources/instance.py b/heat/engine/resources/instance.py
index c76782f21634c9b8f6d0b3b3b0360e0081bf5b73..883be60aed900b86f31093252205f5459de94b97 100644 (file)
--- a/heat/engine/resources/instance.py
+++ b/heat/engine/resources/instance.py
@@ -380,8 +380,10 @@ class Instance(resource.Resource):
     def detach_volumes(self):
         server_id = self.resource_id
         for vol in self.properties['Volumes']:
-            self.stack.clients.detach_volume_from_instance(server_id,
-                                                           vol['VolumeId'])
+            detach_task = volume.VolumeDetachTask(self.stack,
+                                                  self.resource_id,
+                                                  vol['VolumeId'])
+            scheduler.TaskRunner(detach_task)()
 
     def handle_update(self, json_snippet):
         status = self.UPDATE_REPLACE

diff --git a/heat/engine/resources/volume.py b/heat/engine/resources/volume.py
index 8c78846e768661eca2a29d161891668d48a780ff..dda29f49cf292e908ca4c15221d3ec64bb546ea6 100644 (file)
--- a/heat/engine/resources/volume.py
+++ b/heat/engine/resources/volume.py
@@ -168,6 +168,67 @@ class VolumeAttachTask(object):
         logger.info('%s - complete' % str(self))
 
 
+class VolumeDetachTask(object):
+    """A task for attaching a volume to a Nova server."""
+
+    def __init__(self, stack, server_id, volume_id):
+        """
+        Initialise with the stack (for obtaining the clients), and the IDs of
+        the server and volume.
+        """
+        self.clients = stack.clients
+        self.server_id = server_id
+        self.volume_id = volume_id
+
+    def __str__(self):
+        """Return a human-readable string description of the task."""
+        return 'Detaching Volume %s from Instance %s' % (self.volume_id,
+                                                         self.server_id)
+
+    def __repr__(self):
+        """Return a brief string description of the task."""
+        return '%s(%s -/> %s)' % (type(self).__name__,
+                                  self.volume_id,
+                                  self.server_id)
+
+    def __call__(self):
+        """Return a co-routine which runs the task."""
+        logger.debug(str(self))
+
+        try:
+            vol = self.clients.cinder().volumes.get(self.volume_id)
+        except clients.cinder_exceptions.NotFound:
+            logger.warning('%s - volume not found' % str(self))
+            return
+
+        server_api = self.clients.nova().volumes
+
+        try:
+            server_api.delete_server_volume(self.server_id, self.volume_id)
+        except clients.novaclient.exceptions.NotFound:
+            logger.warning('%s - not found' % str(self))
+
+        yield
+
+        try:
+            vol.get()
+            while vol.status == 'in-use':
+                logger.debug('%s - volume still in use' % str(self))
+                yield
+
+                try:
+                    server_api.delete_server_volume(self.server_id,
+                                                    self.volume_id)
+                except clients.novaclient.exceptions.NotFound:
+                    pass
+                vol.get()
+
+            logger.info('%s - status: %s' % (str(self), vol.status))
+
+        except clients.cinderclient.exceptions.NotFound:
+            logger.warning('%s - volume not found' % str(self))
+
+
 class VolumeAttachment(resource.Resource):
     properties_schema = {'InstanceId': {'Type': 'String',
                                         'Required': True},
@@ -195,7 +256,8 @@ class VolumeAttachment(resource.Resource):
     def handle_delete(self):
         server_id = self.properties[self._instance_property]
         volume_id = self.properties[self._volume_property]
-        self.stack.clients.detach_volume_from_instance(server_id, volume_id)
+        detach_task = VolumeDetachTask(self.stack, server_id, volume_id)
+        scheduler.TaskRunner(detach_task)()
 
 
 class CinderVolume(Volume):

diff --git a/heat/tests/test_volume.py b/heat/tests/test_volume.py
index e84073d09fcbc3b4eecc9c63df89809fa58c5c7e..9dbe45704bfb37c9d5a5603d223e4a4cf0e8bbba 100644 (file)
--- a/heat/tests/test_volume.py
+++ b/heat/tests/test_volume.py
@@ -13,7 +13,6 @@
 #    under the License.
 
 
-import eventlet
 import mox
 import json
 
@@ -83,7 +82,6 @@ class VolumeTest(HeatTestCase):
         self.m.StubOutWithMock(self.cinder_fc.volumes, 'delete')
         self.m.StubOutWithMock(self.fc.volumes, 'create_server_volume')
         self.m.StubOutWithMock(self.fc.volumes, 'delete_server_volume')
-        self.m.StubOutWithMock(eventlet, 'sleep')
         self.m.StubOutWithMock(scheduler.TaskRunner, '_sleep')
         setup_dummy_db()
 
@@ -239,7 +237,6 @@ class VolumeTest(HeatTestCase):
         self.fc.volumes.delete_server_volume('WikiDatabase',
                                              'vol-123').AndReturn(None)
         self.cinder_fc.volumes.get('vol-123').AndReturn(fva)
-        eventlet.sleep(mox.IsA(int)).AndReturn(None)
 
         self.m.ReplayAll()
 
@@ -535,7 +532,6 @@ class VolumeTest(HeatTestCase):
         self.fc.volumes.delete_server_volume('WikiDatabase',
                                              'vol-123').AndReturn(None)
         self.cinder_fc.volumes.get('vol-123').AndReturn(fva)
-        eventlet.sleep(mox.IsA(int)).AndReturn(None)
 
         self.m.ReplayAll()