Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Thingee committed May 31, 2013
1 parent f30afa6 commit 8829bb1
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 0 deletions.
3 changes: 3 additions & 0 deletions cinder/api/v2/volumes.py
Expand Up @@ -175,6 +175,9 @@ def delete(self, req, id):
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)
Expand Down
10 changes: 10 additions & 0 deletions cinder/tests/api/v2/test_volumes.py
Expand Up @@ -658,6 +658,16 @@ def test_volume_delete(self):
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)

Expand Down
18 changes: 18 additions & 0 deletions cinder/tests/test_volume.py
Expand Up @@ -561,6 +561,24 @@ def test_force_delete_volume(self):
# 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()
Expand Down
4 changes: 4 additions & 0 deletions cinder/volume/api.py
Expand Up @@ -320,6 +320,10 @@ def delete(self, context, volume, force=False):
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)
Expand Down

0 comments on commit 8829bb1

Please sign in to comment.