Remove API races from delete methods
This patch changes delete API methods to use compare-and-swap updates in order to remove potential races. Updated methods are: - delete - delete_snapshot Specs: https://review.openstack.org/232599/ Implements: blueprint cinder-volume-active-active-support Closes-Bug: #1490944 Change-Id: Ic09825b27774fa4b0ab2b7e60577ecfb3640bcf2
This commit is contained in:
Gorka Eguileor
parent
c7bfd636b3
commit
22b4300253
|
|
@ -19,7 +19,7 @@ from webob import exc
|
|||
from cinder.api import extensions
|
||||
from cinder.api.openstack import wsgi
|
||||
from cinder import exception
|
||||
from cinder.i18n import _, _LI
|
||||
from cinder.i18n import _LI
|
||||
from cinder import volume
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
|
@ -58,9 +58,6 @@ class VolumeUnmanageController(wsgi.Controller):
|
|||
self.volume_api.delete(context, vol, unmanage_only=True)
|
||||
except exception.VolumeNotFound as error:
|
||||
raise exc.HTTPNotFound(explanation=error.msg)
|
||||
except exception.VolumeAttached:
|
||||
msg = _("Volume cannot be deleted while in attached state")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
return webob.Response(status_int=202)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -204,9 +204,6 @@ class VolumeController(wsgi.Controller):
|
|||
self.volume_api.delete(context, volume)
|
||||
except exception.VolumeNotFound as error:
|
||||
raise exc.HTTPNotFound(explanation=error.msg)
|
||||
except exception.VolumeAttached:
|
||||
msg = _("Volume cannot be deleted while in attached state")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
return webob.Response(status_int=202)
|
||||
|
||||
@wsgi.serializers(xml=VolumesTemplate)
|
||||
|
|
|
|||
|
|
@ -275,6 +275,10 @@ def volume_update_status_based_on_attachment(context, volume_id):
|
|||
return IMPL.volume_update_status_based_on_attachment(context, volume_id)
|
||||
|
||||
|
||||
def volume_has_snapshots_filter():
|
||||
return IMPL.volume_has_snapshots_filter()
|
||||
|
||||
|
||||
def volume_has_attachments_filter():
|
||||
return IMPL.volume_has_attachments_filter()
|
||||
|
||||
|
|
|
|||
|
|
@ -1830,6 +1830,12 @@ def volume_update_status_based_on_attachment(context, volume_id):
|
|||
return volume_ref
|
||||
|
||||
|
||||
def volume_has_snapshots_filter():
|
||||
return sql.exists().where(
|
||||
and_(models.Volume.id == models.Snapshot.volume_id,
|
||||
~models.Snapshot.deleted))
|
||||
|
||||
|
||||
def volume_has_attachments_filter():
|
||||
return sql.exists().where(
|
||||
and_(models.Volume.id == models.VolumeAttachment.volume_id,
|
||||
|
|
|
|||
|
|
@ -84,21 +84,23 @@ class SnapshotUnmanageTest(test.TestCase):
|
|||
res = req.get_response(app())
|
||||
return res
|
||||
|
||||
@mock.patch('cinder.db.conditional_update', return_value=1)
|
||||
@mock.patch('cinder.db.snapshot_update')
|
||||
@mock.patch('cinder.objects.volume.Volume.get_by_id')
|
||||
@mock.patch('cinder.db.volume_get')
|
||||
@mock.patch('cinder.objects.Volume.get_by_id')
|
||||
@mock.patch('cinder.volume.rpcapi.VolumeAPI.delete_snapshot')
|
||||
def test_unmanage_snapshot_ok(self, mock_rpcapi, mock_db,
|
||||
mock_volume_get_by_id, mock_db_update):
|
||||
def test_unmanage_snapshot_ok(self, mock_rpcapi, mock_volume_get_by_id,
|
||||
mock_db_update, mock_conditional_update):
|
||||
"""Return success for valid and unattached volume."""
|
||||
ctxt = context.RequestContext('admin', 'fake', True)
|
||||
volume = fake_volume.fake_volume_obj(ctxt, id='fake_volume_id')
|
||||
mock_volume_get_by_id.return_value = volume
|
||||
res = self._get_resp(snapshot_id)
|
||||
|
||||
self.assertEqual(1, mock_db.call_count)
|
||||
self.assertEqual(2, len(mock_db.call_args[0]), mock_db.call_args)
|
||||
self.assertEqual('fake_volume_id', mock_db.call_args[0][1])
|
||||
self.assertEqual(1, mock_volume_get_by_id.call_count)
|
||||
self.assertEqual(2, len(mock_volume_get_by_id.call_args[0]),
|
||||
mock_volume_get_by_id.call_args)
|
||||
self.assertEqual('fake_volume_id',
|
||||
mock_volume_get_by_id.call_args[0][1])
|
||||
|
||||
self.assertEqual(1, mock_rpcapi.call_count)
|
||||
self.assertEqual(3, len(mock_rpcapi.call_args[0]))
|
||||
|
|
|
|||
|
|
@ -17,90 +17,13 @@ from oslo_serialization import jsonutils
|
|||
import webob
|
||||
|
||||
from cinder import context
|
||||
from cinder import exception
|
||||
from cinder import db
|
||||
from cinder import objects
|
||||
from cinder import test
|
||||
from cinder.tests.unit.api import fakes
|
||||
from cinder.tests.unit import fake_snapshot
|
||||
from cinder.tests.unit import fake_volume
|
||||
from cinder.tests.unit import utils
|
||||
|
||||
|
||||
# This list of fake volumes is used by our tests. Each is configured in a
|
||||
# slightly different way, and includes only the properties that are required
|
||||
# for these particular tests to function correctly.
|
||||
snapshot_vol_id = 'ffffffff-0000-ffff-0000-fffffffffffd'
|
||||
detached_vol_id = 'ffffffff-0000-ffff-0000-fffffffffffe'
|
||||
attached_vol_id = 'ffffffff-0000-ffff-0000-ffffffffffff'
|
||||
bad_vol_id = 'ffffffff-0000-ffff-0000-fffffffffff0'
|
||||
|
||||
vols = {snapshot_vol_id: {'id': snapshot_vol_id,
|
||||
'status': 'available',
|
||||
'attach_status': 'detached',
|
||||
'host': 'fake_host',
|
||||
'project_id': 'fake_project',
|
||||
'migration_status': None,
|
||||
'consistencygroup_id': None,
|
||||
'encryption_key_id': None},
|
||||
detached_vol_id: {'id': detached_vol_id,
|
||||
'status': 'available',
|
||||
'attach_status': 'detached',
|
||||
'host': 'fake_host',
|
||||
'project_id': 'fake_project',
|
||||
'migration_status': None,
|
||||
'consistencygroup_id': None,
|
||||
'encryption_key_id': None},
|
||||
attached_vol_id: {'id': attached_vol_id,
|
||||
'status': 'available',
|
||||
'attach_status': 'attached',
|
||||
'host': 'fake_host',
|
||||
'project_id': 'fake_project',
|
||||
'migration_status': None,
|
||||
'consistencygroup_id': None,
|
||||
'encryption_key_id': None}
|
||||
}
|
||||
|
||||
|
||||
def app():
|
||||
# no auth, just let environ['cinder.context'] pass through
|
||||
api = fakes.router.APIRouter()
|
||||
mapper = fakes.urlmap.URLMap()
|
||||
mapper['/v2'] = api
|
||||
return mapper
|
||||
|
||||
|
||||
def api_get(self, context, volume_id):
|
||||
"""Replacement for cinder.volume.api.API.get.
|
||||
|
||||
We stub the cinder.volume.api.API.get method to check for the existence
|
||||
of volume_id in our list of fake volumes and raise an exception if the
|
||||
specified volume ID is not in our list.
|
||||
"""
|
||||
vol = vols.get(volume_id, None)
|
||||
|
||||
if not vol:
|
||||
raise exception.VolumeNotFound(volume_id)
|
||||
|
||||
return fake_volume.fake_volume_obj(context, **vol)
|
||||
|
||||
|
||||
def db_snapshot_get_all_for_volume(context, volume_id):
|
||||
"""Replacement for cinder.db.snapshot_get_all_for_volume.
|
||||
|
||||
We stub the cinder.db.snapshot_get_all_for_volume method because when we
|
||||
go to unmanage a volume, the code checks for snapshots and won't unmanage
|
||||
volumes with snapshots. For these tests, only the snapshot_vol_id reports
|
||||
any snapshots. The delete code just checks for array length, doesn't
|
||||
inspect the contents.
|
||||
"""
|
||||
if volume_id == snapshot_vol_id:
|
||||
db_snapshot = {'volume_id': volume_id}
|
||||
snapshot = fake_snapshot.fake_db_snapshot(**db_snapshot)
|
||||
return [snapshot]
|
||||
return []
|
||||
|
||||
|
||||
@mock.patch('cinder.volume.api.API.get', api_get)
|
||||
@mock.patch('cinder.db.snapshot_get_all_for_volume',
|
||||
db_snapshot_get_all_for_volume)
|
||||
class VolumeUnmanageTest(test.TestCase):
|
||||
"""Test cases for cinder/api/contrib/volume_unmanage.py
|
||||
|
||||
|
|
@ -117,50 +40,54 @@ class VolumeUnmanageTest(test.TestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(VolumeUnmanageTest, self).setUp()
|
||||
self.ctxt = context.RequestContext('admin', 'fake_project', True)
|
||||
|
||||
api = fakes.router.APIRouter()
|
||||
self.app = fakes.urlmap.URLMap()
|
||||
self.app['/v2'] = api
|
||||
|
||||
def _get_resp(self, volume_id):
|
||||
"""Helper to build an os-unmanage req for the specified volume_id."""
|
||||
req = webob.Request.blank('/v2/fake/volumes/%s/action' % volume_id)
|
||||
req = webob.Request.blank('/v2/%s/volumes/%s/action' %
|
||||
(self.ctxt.project_id, volume_id))
|
||||
req.method = 'POST'
|
||||
req.headers['Content-Type'] = 'application/json'
|
||||
req.environ['cinder.context'] = context.RequestContext('admin',
|
||||
'fake',
|
||||
True)
|
||||
req.environ['cinder.context'] = self.ctxt
|
||||
body = {'os-unmanage': ''}
|
||||
req.body = jsonutils.dump_as_bytes(body)
|
||||
res = req.get_response(app())
|
||||
res = req.get_response(self.app)
|
||||
return res
|
||||
|
||||
@mock.patch('cinder.db.volume_update')
|
||||
@mock.patch('cinder.volume.rpcapi.VolumeAPI.delete_volume')
|
||||
def test_unmanage_volume_ok(self, mock_rpcapi, mock_db):
|
||||
def test_unmanage_volume_ok(self, mock_rpcapi):
|
||||
"""Return success for valid and unattached volume."""
|
||||
res = self._get_resp(detached_vol_id)
|
||||
|
||||
# volume_update is (context, id, new_data)
|
||||
self.assertEqual(1, mock_db.call_count)
|
||||
self.assertEqual(3, len(mock_db.call_args[0]), mock_db.call_args)
|
||||
self.assertEqual(detached_vol_id, mock_db.call_args[0][1])
|
||||
|
||||
# delete_volume is (context, status, unmanageOnly)
|
||||
self.assertEqual(1, mock_rpcapi.call_count)
|
||||
self.assertEqual(3, len(mock_rpcapi.call_args[0]))
|
||||
self.assertTrue(mock_rpcapi.call_args[0][2])
|
||||
|
||||
vol = utils.create_volume(self.ctxt)
|
||||
res = self._get_resp(vol.id)
|
||||
self.assertEqual(202, res.status_int, res)
|
||||
|
||||
mock_rpcapi.called_once_with(self.ctxt, mock.ANY, unmanage_only=True)
|
||||
vol = objects.volume.Volume.get_by_id(self.ctxt, vol.id)
|
||||
self.assertEqual('deleting', vol.status)
|
||||
db.volume_destroy(self.ctxt, vol.id)
|
||||
|
||||
def test_unmanage_volume_bad_volume_id(self):
|
||||
"""Return 404 if the volume does not exist."""
|
||||
res = self._get_resp(bad_vol_id)
|
||||
res = self._get_resp('nonexistent-volume-id')
|
||||
self.assertEqual(404, res.status_int, res)
|
||||
|
||||
def test_unmanage_volume_attached_(self):
|
||||
def test_unmanage_volume_attached(self):
|
||||
"""Return 400 if the volume exists but is attached."""
|
||||
res = self._get_resp(attached_vol_id)
|
||||
vol = utils.create_volume(self.ctxt, status='in-use',
|
||||
attach_status='attached')
|
||||
res = self._get_resp(vol.id)
|
||||
self.assertEqual(400, res.status_int, res)
|
||||
db.volume_destroy(self.ctxt, vol.id)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_unmanage_volume_with_snapshots(self, metadata_get):
|
||||
def test_unmanage_volume_with_snapshots(self):
|
||||
"""Return 400 if the volume exists but has snapshots."""
|
||||
res = self._get_resp(snapshot_vol_id)
|
||||
vol = utils.create_volume(self.ctxt)
|
||||
snap = utils.create_snapshot(self.ctxt, vol.id)
|
||||
res = self._get_resp(vol.id)
|
||||
self.assertEqual(400, res.status_int, res)
|
||||
db.volume_destroy(self.ctxt, vol.id)
|
||||
db.snapshot_destroy(self.ctxt, snap.id)
|
||||
|
|
|
|||
|
|
@ -1289,10 +1289,10 @@ class VolumeApiTest(test.TestCase):
|
|||
self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v2/volumes/1')
|
||||
exp = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
exp = self.assertRaises(exception.VolumeAttached,
|
||||
self.controller.delete,
|
||||
req, 1)
|
||||
expect_msg = "Volume cannot be deleted while in attached state"
|
||||
expect_msg = "Volume 1 is still attached, detach volume first."
|
||||
self.assertEqual(expect_msg, six.text_type(exp))
|
||||
|
||||
def test_volume_delete_no_volume(self):
|
||||
|
|
|
|||
|
|
@ -133,6 +133,7 @@ class BaseVolumeTestCase(test.TestCase):
|
|||
'pools': {}}
|
||||
# keep ordered record of what we execute
|
||||
self.called = []
|
||||
self.volume_api = cinder.volume.api.API()
|
||||
|
||||
def _cleanup(self):
|
||||
try:
|
||||
|
|
@ -936,25 +937,17 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
|
||||
@mock.patch.object(keymgr, 'API', new=fake_keymgr.fake_api)
|
||||
def test_create_delete_volume_with_encrypted_volume_type(self):
|
||||
ctxt = context.get_admin_context()
|
||||
|
||||
db.volume_type_create(ctxt,
|
||||
{'id': '61298380-0c12-11e3-bfd6-4b48424183be',
|
||||
'name': 'LUKS'})
|
||||
db_vol_type = db.volume_type_create(self.context,
|
||||
{'id': 'type-id', 'name': 'LUKS'})
|
||||
db.volume_type_encryption_create(
|
||||
ctxt,
|
||||
'61298380-0c12-11e3-bfd6-4b48424183be',
|
||||
self.context, 'type-id',
|
||||
{'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER})
|
||||
|
||||
volume_api = cinder.volume.api.API()
|
||||
|
||||
db_vol_type = db.volume_type_get_by_name(ctxt, 'LUKS')
|
||||
|
||||
volume = volume_api.create(self.context,
|
||||
1,
|
||||
'name',
|
||||
'description',
|
||||
volume_type=db_vol_type)
|
||||
volume = self.volume_api.create(self.context,
|
||||
1,
|
||||
'name',
|
||||
'description',
|
||||
volume_type=db_vol_type)
|
||||
|
||||
self.assertIsNotNone(volume.get('encryption_key_id', None))
|
||||
self.assertEqual(db_vol_type.get('id'), volume['volume_type_id'])
|
||||
|
|
@ -962,7 +955,8 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
|
||||
volume['host'] = 'fake_host'
|
||||
volume['status'] = 'available'
|
||||
volume_api.delete(self.context, volume)
|
||||
db.volume_update(self.context, volume['id'], {'status': 'available'})
|
||||
self.volume_api.delete(self.context, volume)
|
||||
|
||||
volume = db.volume_get(self.context, volume['id'])
|
||||
self.assertEqual('deleting', volume['status'])
|
||||
|
|
@ -2989,10 +2983,11 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
pass
|
||||
|
||||
@staticmethod
|
||||
def _create_snapshot(volume_id, size=1, metadata=None):
|
||||
def _create_snapshot(volume_id, size=1, metadata=None, ctxt=None,
|
||||
**kwargs):
|
||||
"""Create a snapshot object."""
|
||||
metadata = metadata or {}
|
||||
snap = objects.Snapshot(context.get_admin_context())
|
||||
snap = objects.Snapshot(ctxt or context.get_admin_context())
|
||||
snap.volume_size = size
|
||||
snap.user_id = 'fake'
|
||||
snap.project_id = 'fake'
|
||||
|
|
@ -3000,6 +2995,8 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
snap.status = "creating"
|
||||
if metadata is not None:
|
||||
snap.metadata = metadata
|
||||
snap.update(kwargs)
|
||||
|
||||
snap.create()
|
||||
return snap
|
||||
|
||||
|
|
@ -3171,16 +3168,12 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
def _test_cannot_delete_volume(self, status):
|
||||
"""Test volume can't be deleted in invalid stats."""
|
||||
# create a volume and assign to host
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
self.volume.create_volume(self.context, volume['id'])
|
||||
volume['status'] = status
|
||||
volume['host'] = 'fakehost'
|
||||
|
||||
volume_api = cinder.volume.api.API()
|
||||
volume = tests_utils.create_volume(self.context, CONF.host,
|
||||
status=status)
|
||||
|
||||
# 'in-use' status raises InvalidVolume
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
volume_api.delete,
|
||||
self.volume_api.delete,
|
||||
self.context,
|
||||
volume)
|
||||
|
||||
|
|
@ -3190,20 +3183,17 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
def test_force_delete_volume(self):
|
||||
"""Test volume can be forced to delete."""
|
||||
# create a volume and assign to host
|
||||
self.volume_params['status'] = 'error_deleting'
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
self.volume.create_volume(self.context, volume['id'])
|
||||
volume['status'] = 'error_deleting'
|
||||
|
||||
volume_api = cinder.volume.api.API()
|
||||
|
||||
# 'error_deleting' volumes can't be deleted
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
volume_api.delete,
|
||||
self.volume_api.delete,
|
||||
self.context,
|
||||
volume)
|
||||
|
||||
# delete with force
|
||||
volume_api.delete(self.context, volume, force=True)
|
||||
self.volume_api.delete(self.context, volume, force=True)
|
||||
|
||||
# status is deleting
|
||||
volume = objects.Volume.get_by_id(context.get_admin_context(),
|
||||
|
|
@ -3215,21 +3205,17 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
|
||||
def test_cannot_force_delete_attached_volume(self):
|
||||
"""Test volume can't be force delete in attached state."""
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
self.volume.create_volume(self.context, volume['id'])
|
||||
volume['status'] = 'in-use'
|
||||
volume['attach_status'] = 'attached'
|
||||
volume['host'] = 'fakehost'
|
||||
volume = tests_utils.create_volume(self.context, CONF.host,
|
||||
status='in-use',
|
||||
attach_status = 'attached')
|
||||
|
||||
volume_api = cinder.volume.api.API()
|
||||
|
||||
self.assertRaises(exception.VolumeAttached,
|
||||
volume_api.delete,
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
self.volume_api.delete,
|
||||
self.context,
|
||||
volume,
|
||||
force=True)
|
||||
|
||||
self.volume.delete_volume(self.context, volume['id'])
|
||||
db.volume_destroy(self.context, volume.id)
|
||||
|
||||
def test_cannot_delete_volume_with_snapshots(self):
|
||||
"""Test volume can't be deleted with dependent snapshots."""
|
||||
|
|
@ -3255,22 +3241,20 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
|
||||
def test_can_delete_errored_snapshot(self):
|
||||
"""Test snapshot can be created and deleted."""
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
self.volume.create_volume(self.context, volume['id'])
|
||||
snapshot = self._create_snapshot(volume['id'], size=volume['size'])
|
||||
self.volume.create_snapshot(self.context, volume['id'], snapshot)
|
||||
|
||||
volume_api = cinder.volume.api.API()
|
||||
|
||||
snapshot.status = 'badstatus'
|
||||
volume = tests_utils.create_volume(self.context, CONF.host)
|
||||
snapshot = self._create_snapshot(volume.id, size=volume['size'],
|
||||
ctxt=self.context, status='bad')
|
||||
self.assertRaises(exception.InvalidSnapshot,
|
||||
volume_api.delete_snapshot,
|
||||
self.volume_api.delete_snapshot,
|
||||
self.context,
|
||||
snapshot)
|
||||
|
||||
snapshot.status = 'error'
|
||||
self.volume.delete_snapshot(self.context, snapshot)
|
||||
self.volume.delete_volume(self.context, volume['id'])
|
||||
snapshot.save()
|
||||
self.volume_api.delete_snapshot(self.context, snapshot)
|
||||
|
||||
self.assertEqual('deleting', snapshot.status)
|
||||
self.volume.delete_volume(self.context, volume.id)
|
||||
|
||||
def test_create_snapshot_force(self):
|
||||
"""Test snapshot in use can be created forcibly."""
|
||||
|
|
@ -4942,12 +4926,11 @@ class VolumeMigrationTestCase(VolumeTestCase):
|
|||
class ConsistencyGroupTestCase(BaseVolumeTestCase):
|
||||
def test_delete_volume_in_consistency_group(self):
|
||||
"""Test deleting a volume that's tied to a consistency group fails."""
|
||||
volume_api = cinder.volume.api.API()
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
consistencygroup_id = '12345678-1234-5678-1234-567812345678'
|
||||
volume = db.volume_update(self.context, volume['id'],
|
||||
{'status': 'available',
|
||||
volume_api = cinder.volume.api.API()
|
||||
self.volume_params.update({'status': 'available',
|
||||
'consistencygroup_id': consistencygroup_id})
|
||||
volume = tests_utils.create_volume(self.context, **self.volume_params)
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
volume_api.delete, self.context, volume)
|
||||
|
||||
|
|
|
|||
|
|
@ -352,22 +352,21 @@ class API(base.Base):
|
|||
|
||||
@wrap_check_policy
|
||||
def delete(self, context, volume, force=False, unmanage_only=False):
|
||||
if context.is_admin and context.project_id != volume['project_id']:
|
||||
project_id = volume['project_id']
|
||||
if context.is_admin and context.project_id != volume.project_id:
|
||||
project_id = volume.project_id
|
||||
else:
|
||||
project_id = context.project_id
|
||||
|
||||
volume_id = volume['id']
|
||||
if not volume['host']:
|
||||
if not volume.host:
|
||||
volume_utils.notify_about_volume_usage(context,
|
||||
volume, "delete.start")
|
||||
# NOTE(vish): scheduling failed, so delete it
|
||||
# Note(zhiteng): update volume quota reservation
|
||||
try:
|
||||
reserve_opts = {'volumes': -1, 'gigabytes': -volume['size']}
|
||||
reserve_opts = {'volumes': -1, 'gigabytes': -volume.size}
|
||||
QUOTAS.add_volume_type_opts(context,
|
||||
reserve_opts,
|
||||
volume['volume_type_id'])
|
||||
volume.volume_type_id)
|
||||
reservations = QUOTAS.reserve(context,
|
||||
project_id=project_id,
|
||||
**reserve_opts)
|
||||
|
|
@ -384,51 +383,35 @@ class API(base.Base):
|
|||
volume, "delete.end")
|
||||
LOG.info(_LI("Delete volume request issued successfully."),
|
||||
resource={'type': 'volume',
|
||||
'id': volume_id})
|
||||
'id': volume.id})
|
||||
return
|
||||
if volume['attach_status'] == "attached":
|
||||
# Volume is still attached, need to detach first
|
||||
LOG.info(_LI('Unable to delete volume: %s, '
|
||||
'volume is attached.'), volume['id'])
|
||||
raise exception.VolumeAttached(volume_id=volume_id)
|
||||
|
||||
if not force and volume['status'] not in ["available", "error",
|
||||
"error_restoring",
|
||||
"error_extending"]:
|
||||
msg = _("Volume status must be available or error, "
|
||||
"but current status is: %s.") % volume['status']
|
||||
LOG.info(_LI('Unable to delete volume: %(vol_id)s, '
|
||||
'volume must be available or '
|
||||
'error, but is %(vol_status)s.'),
|
||||
{'vol_id': volume['id'],
|
||||
'vol_status': volume['status']})
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
# Build required conditions for conditional update
|
||||
expected = {'attach_status': db.Not('attached'),
|
||||
'migration_status': self.AVAILABLE_MIGRATION_STATUS,
|
||||
'consistencygroup_id': None}
|
||||
|
||||
if self._is_volume_migrating(volume):
|
||||
# Volume is migrating, wait until done
|
||||
LOG.info(_LI('Unable to delete volume: %s, '
|
||||
'volume is currently migrating.'), volume['id'])
|
||||
msg = _("Volume cannot be deleted while migrating")
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
# If not force deleting we have status conditions
|
||||
if not force:
|
||||
expected['status'] = ('available', 'error', 'error_restoring',
|
||||
'error_extending')
|
||||
|
||||
if volume['consistencygroup_id'] is not None:
|
||||
msg = _("Volume cannot be deleted while in a consistency group.")
|
||||
LOG.info(_LI('Unable to delete volume: %s, '
|
||||
'volume is currently part of a '
|
||||
'consistency group.'), volume['id'])
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
# Volume cannot have snapshots if we want to delete it
|
||||
filters = [~db.volume_has_snapshots_filter()]
|
||||
values = {'status': 'deleting', 'terminated_at': timeutils.utcnow()}
|
||||
|
||||
snapshots = objects.SnapshotList.get_all_for_volume(context,
|
||||
volume_id)
|
||||
if len(snapshots):
|
||||
LOG.info(_LI('Unable to delete volume: %s, '
|
||||
'volume currently has snapshots.'), volume['id'])
|
||||
msg = _("Volume still has %d dependent "
|
||||
"snapshots.") % len(snapshots)
|
||||
result = volume.conditional_update(values, expected, filters)
|
||||
|
||||
if not result:
|
||||
status = utils.build_or_str(expected.get('status'),
|
||||
_(' status must be %s and '))
|
||||
msg = _('Volume%s must not be migrating, attached, belong to a '
|
||||
'consistency group or have snapshots.') % status
|
||||
LOG.info(msg)
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
|
||||
cache = image_cache.ImageVolumeCache(self.db, self)
|
||||
entry = cache.get_by_image_volume(context, volume_id)
|
||||
entry = cache.get_by_image_volume(context, volume.id)
|
||||
if entry:
|
||||
cache.evict(context, entry)
|
||||
|
||||
|
|
@ -443,10 +426,6 @@ class API(base.Base):
|
|||
msg = _("Unable to delete encrypted volume: %s.") % e.msg
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
|
||||
volume.status = 'deleting'
|
||||
volume.terminated_at = timeutils.utcnow()
|
||||
volume.save()
|
||||
|
||||
self.volume_rpcapi.delete_volume(context, volume, unmanage_only)
|
||||
LOG.info(_LI("Delete volume request issued successfully."),
|
||||
resource=volume)
|
||||
|
|
@ -949,29 +928,24 @@ class API(base.Base):
|
|||
@wrap_check_policy
|
||||
def delete_snapshot(self, context, snapshot, force=False,
|
||||
unmanage_only=False):
|
||||
if not force and snapshot.status not in ["available", "error"]:
|
||||
LOG.error(_LE('Unable to delete snapshot: %(snap_id)s, '
|
||||
'due to invalid status. '
|
||||
'Status must be available or '
|
||||
'error, not %(snap_status)s.'),
|
||||
{'snap_id': snapshot.id,
|
||||
'snap_status': snapshot.status})
|
||||
msg = _("Volume Snapshot status must be available or error.")
|
||||
raise exception.InvalidSnapshot(reason=msg)
|
||||
cgsnapshot_id = snapshot.cgsnapshot_id
|
||||
if cgsnapshot_id:
|
||||
msg = _('Unable to delete snapshot %s because it is part of a '
|
||||
'consistency group.') % snapshot.id
|
||||
# Build required conditions for conditional update
|
||||
expected = {'cgsnapshot_id': None}
|
||||
# If not force deleting we have status conditions
|
||||
if not force:
|
||||
expected['status'] = ('available', 'error')
|
||||
|
||||
result = snapshot.conditional_update({'status': 'deleting'}, expected)
|
||||
if not result:
|
||||
status = utils.build_or_str(expected.get('status'),
|
||||
_(' status must be %s and '))
|
||||
msg = (_('Snapshot%s must not be part of a consistency group.') %
|
||||
status)
|
||||
LOG.error(msg)
|
||||
raise exception.InvalidSnapshot(reason=msg)
|
||||
|
||||
snapshot_obj = self.get_snapshot(context, snapshot.id)
|
||||
snapshot_obj.status = 'deleting'
|
||||
snapshot_obj.save()
|
||||
|
||||
volume = self.db.volume_get(context, snapshot_obj.volume_id)
|
||||
self.volume_rpcapi.delete_snapshot(context, snapshot_obj,
|
||||
volume['host'],
|
||||
# Make RPC call to the right host
|
||||
volume = objects.Volume.get_by_id(context, snapshot.volume_id)
|
||||
self.volume_rpcapi.delete_snapshot(context, snapshot, volume.host,
|
||||
unmanage_only=unmanage_only)
|
||||
LOG.info(_LI("Snapshot delete request issued successfully."),
|
||||
resource=snapshot)
|
||||
|
|
|
|||
Loading…
Reference in New Issue