From: Mike Perez <thingee@gmail.com>
Date: Fri, 31 May 2013 07:11:24 +0000 (-0700)
Subject: Prevent force delete if the volume is attached
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=8829bb1a1d726bd6b1b6b623c0ac498d1ec05d41;p=openstack-build%2Fcinder-build.git

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 720b7cbe1..d56fe6b7c 100644
--- 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 9b08ab376..7264c30a4 100644
--- 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 f9ec01302..39874c81d 100644
--- 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 68550b5e3..d24e8a0e7 100644
--- 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)