From: Mike Perez <thingee@gmail.com>
Date: Thu, 22 May 2014 21:29:06 +0000 (-0700)
Subject: Handle volumes no longer existing in resume delete
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b8a064f10a4dad097ef40f401015897cf9b424c3;p=openstack-build%2Fcinder-build.git

Handle volumes no longer existing in resume delete

init_host resumes deletes for volumes with a status of 'deleting'. It
does these sequentially, and sometimes a volume could be deleted under
it, causing a bad stacktrace. This will gracefully handle those
situations and move on. Added coverage for resume volume delete being
successful as well.

Change-Id: I5b30265609b65804f2da77d9daa84b640de7939c
Closes-Bug: #1322340
---

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index a0016af73..f8fd46ea4 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -154,6 +154,15 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual(volume['status'], "error")
         self.volume.delete_volume(self.context, volume_id)
 
+    def test_init_host_resumes_deletes(self):
+        """init_host will resume deleting volume in deleting status."""
+        volume = tests_utils.create_volume(self.context, status='deleting',
+                                           size=0, host=CONF.host)
+        volume_id = volume['id']
+        self.volume.init_host()
+        self.assertRaises(exception.VolumeNotFound, db.volume_get,
+                          context.get_admin_context(), volume_id)
+
     @mock.patch.object(QUOTAS, 'reserve')
     @mock.patch.object(QUOTAS, 'commit')
     @mock.patch.object(QUOTAS, 'rollback')
@@ -521,6 +530,14 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertRaises(exception.NotFound, db.volume_get,
                           self.context, volume['id'])
 
+    @mock.patch.object(db, 'volume_get', side_effect=exception.VolumeNotFound(
+                       volume_id='12345678-1234-5678-1234-567812345678'))
+    def test_delete_volume_not_found(self, mock_get_volume):
+        """"Test delete volume moves on if the volume does not exist."""
+        volume_id = '12345678-1234-5678-1234-567812345678'
+        self.assertTrue(self.volume.delete_volume(self.context, volume_id))
+        self.assertTrue(mock_get_volume.called)
+
     def test_create_volume_from_snapshot(self):
         """Test volume can be created from a snapshot."""
         volume_src = tests_utils.create_volume(self.context,
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 32fbfa517..049f83a36 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -346,7 +346,15 @@ class VolumeManager(manager.SchedulerDependentManager):
     def delete_volume(self, context, volume_id, unmanage_only=False):
         """Deletes and unexports volume."""
         context = context.elevated()
-        volume_ref = self.db.volume_get(context, volume_id)
+
+        try:
+            volume_ref = self.db.volume_get(context, volume_id)
+        except exception.VolumeNotFound:
+            # NOTE(thingee): It could be possible for a volume to
+            # be deleted when resuming deletes from init_host().
+            LOG.info(_("Tried to delete volume %s, but it no longer exists, "
+                       "moving on") % (volume_id))
+            return True
 
         if context.project_id != volume_ref['project_id']:
             project_id = volume_ref['project_id']