Prevent force delete if the volume is attached

Force deletes were eventually failing on the volume manager layer due to
being in an attached state. This will check that up front to inform the
user that they need to detach first.

Fixes: bug #1164929
Change-Id: I24ade24fd750dc647331ef25b835f45f29c10fd7

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index 720b7cbe1536a01b51619cc798fee8f93ed1b876..d56fe6b7c2aa8bb22ee8dc7574773e1fd3e5f717 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -175,6 +175,9 @@ class VolumeController(wsgi.Controller):
             self.volume_api.delete(context, volume)
         except exception.NotFound:
             raise exc.HTTPNotFound()
+        except exception.VolumeAttached:
+            explanation = 'Volume cannot be deleted while in attached state'
+            raise exc.HTTPBadRequest(explanation=explanation)
         return webob.Response(status_int=202)
 
     @wsgi.serializers(xml=VolumesTemplate)

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 9b08ab376a2ecd1e76930fd9380ec595a9e019f6..7264c30a4977e62c537c2cf2036ad2beae825b00 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -658,6 +658,16 @@ class VolumeApiTest(test.TestCase):
         resp = self.controller.delete(req, 1)
         self.assertEqual(resp.status_int, 202)
 
+    def test_volume_delete_attached(self):
+        def stub_volume_attached(self, context, volume, force=False):
+            raise exception.VolumeAttached(volume_id=volume['id'])
+        self.stubs.Set(volume_api.API, "delete", stub_volume_attached)
+
+        req = fakes.HTTPRequest.blank('/v2/volumes/1')
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.delete,
+                          req, 1)
+
     def test_volume_delete_no_volume(self):
         self.stubs.Set(volume_api.API, "get", stubs.stub_volume_get_notfound)
 

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index f9ec0130268d45574d4628d67cd45ff6dd84d06a..39874c81d6a7577e23a99fa4d61eb12cb58c484c 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -561,6 +561,24 @@ class VolumeTestCase(test.TestCase):
         # clean up
         self.volume.delete_volume(self.context, volume['id'])
 
+    def test_cant_force_delete_attached_volume(self):
+        """Test volume can't be force delete in attached state"""
+        volume = self._create_volume()
+        self.volume.create_volume(self.context, volume['id'])
+        volume['status'] = 'in-use'
+        volume['attach_status'] = 'attached'
+        volume['host'] = 'fakehost'
+
+        volume_api = cinder.volume.api.API()
+
+        self.assertRaises(exception.VolumeAttached,
+                          volume_api.delete,
+                          self.context,
+                          volume,
+                          force=True)
+
+        self.volume.delete_volume(self.context, volume['id'])
+
     def test_cant_delete_volume_with_snapshots(self):
         """Test volume can't be deleted with dependent snapshots."""
         volume = self._create_volume()

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 68550b5e318dac9bd5a64d6bd2cd6fbdaf10beed..d24e8a0e766d292f72a5065da1e026b04e5aadd8 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -320,6 +320,10 @@ class API(base.Base):
             msg = _("Volume status must be available or error")
             raise exception.InvalidVolume(reason=msg)
 
+        if volume['attach_status'] == "attached":
+            # Volume is still attached, need to detach first
+            raise exception.VolumeAttached(volume_id=volume_id)
+
         snapshots = self.db.snapshot_get_all_for_volume(context, volume_id)
         if len(snapshots):
             msg = _("Volume still has %d dependent snapshots") % len(snapshots)