From b8a064f10a4dad097ef40f401015897cf9b424c3 Mon Sep 17 00:00:00 2001 From: Mike Perez Date: Thu, 22 May 2014 14:29:06 -0700 Subject: [PATCH] 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 --- cinder/tests/test_volume.py | 17 +++++++++++++++++ cinder/volume/manager.py | 10 +++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) 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'] -- 2.45.2