Merge "Fix quota updating when admin deletes common user's volume"

This commit is contained in:
Jenkins 2013-03-27 00:45:11 +00:00 committed by Gerrit Code Review
commit 04f880d40a
7 changed files with 167 additions and 69 deletions

@ -680,20 +680,22 @@ def reservation_destroy(context, uuid):
def quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age):
until_refresh, max_age, project_id=None):
"""Check quotas and create appropriate reservations."""
return IMPL.quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age)
until_refresh, max_age, project_id=project_id)
def reservation_commit(context, reservations):
def reservation_commit(context, reservations, project_id=None):
"""Commit quota reservations."""
return IMPL.reservation_commit(context, reservations)
return IMPL.reservation_commit(context, reservations,
project_id=project_id)
def reservation_rollback(context, reservations):
def reservation_rollback(context, reservations, project_id=None):
"""Roll back quota reservations."""
return IMPL.reservation_rollback(context, reservations)
return IMPL.reservation_rollback(context, reservations,
project_id=project_id)
def quota_destroy_all_by_project(context, project_id):

@ -648,12 +648,12 @@ def reservation_destroy(context, uuid):
# code always acquires the lock on quota_usages before acquiring the lock
# on reservations.
def _get_quota_usages(context, session):
def _get_quota_usages(context, session, project_id):
# Broken out for testability
rows = model_query(context, models.QuotaUsage,
read_deleted="no",
session=session).\
filter_by(project_id=context.project_id).\
filter_by(project_id=project_id).\
with_lockmode('update').\
all()
return dict((row.resource, row) for row in rows)
@ -661,12 +661,15 @@ def _get_quota_usages(context, session):
@require_context
def quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age):
until_refresh, max_age, project_id=None):
elevated = context.elevated()
session = get_session()
with session.begin():
if project_id is None:
project_id = context.project_id
# Get the current usages
usages = _get_quota_usages(context, session)
usages = _get_quota_usages(context, session, project_id)
# Handle usage refresh
work = set(deltas.keys())
@ -677,7 +680,7 @@ def quota_reserve(context, resources, quotas, deltas, expire,
refresh = False
if resource not in usages:
usages[resource] = quota_usage_create(elevated,
context.project_id,
project_id,
resource,
0, 0,
until_refresh or None,
@ -700,12 +703,12 @@ def quota_reserve(context, resources, quotas, deltas, expire,
# Grab the sync routine
sync = resources[resource].sync
updates = sync(elevated, context.project_id, session)
updates = sync(elevated, project_id, session)
for res, in_use in updates.items():
# Make sure we have a destination for the usage!
if res not in usages:
usages[res] = quota_usage_create(elevated,
context.project_id,
project_id,
res,
0, 0,
until_refresh or None,
@ -755,7 +758,7 @@ def quota_reserve(context, resources, quotas, deltas, expire,
reservation = reservation_create(elevated,
str(uuid.uuid4()),
usages[resource],
context.project_id,
project_id,
resource, delta, expire,
session=session)
reservations.append(reservation.uuid)
@ -804,10 +807,10 @@ def _quota_reservations(session, context, reservations):
@require_context
def reservation_commit(context, reservations):
def reservation_commit(context, reservations, project_id=None):
session = get_session()
with session.begin():
usages = _get_quota_usages(context, session)
usages = _get_quota_usages(context, session, project_id)
for reservation in _quota_reservations(session, context, reservations):
usage = usages[reservation.resource]
@ -822,10 +825,10 @@ def reservation_commit(context, reservations):
@require_context
def reservation_rollback(context, reservations):
def reservation_rollback(context, reservations, project_id=None):
session = get_session()
with session.begin():
usages = _get_quota_usages(context, session)
usages = _get_quota_usages(context, session, project_id)
for reservation in _quota_reservations(session, context, reservations):
usage = usages[reservation.resource]

@ -174,7 +174,7 @@ class DbQuotaDriver(object):
return quotas
def _get_quotas(self, context, resources, keys, has_sync):
def _get_quotas(self, context, resources, keys, has_sync, project_id=None):
"""
A helper method which retrieves the quotas for the specific
resources identified by keys, and which apply to the current
@ -187,6 +187,9 @@ class DbQuotaDriver(object):
have a sync attribute; if False, indicates
that the resource must NOT have a sync
attribute.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
# Filter resources
@ -205,12 +208,12 @@ class DbQuotaDriver(object):
# Grab and return the quotas (without usages)
quotas = self.get_project_quotas(context, sub_resources,
context.project_id,
project_id,
context.quota_class, usages=False)
return dict((k, v['limit']) for k, v in quotas.items())
def limit_check(self, context, resources, values):
def limit_check(self, context, resources, values, project_id=None):
"""Check simple quota limits.
For limits--those quotas for which there is no usage
@ -230,6 +233,9 @@ class DbQuotaDriver(object):
:param resources: A dictionary of the registered resources.
:param values: A dictionary of the values to check against the
quota.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
# Ensure no value is less than zero
@ -237,9 +243,13 @@ class DbQuotaDriver(object):
if unders:
raise exception.InvalidQuotaValue(unders=sorted(unders))
# If project_id is None, then we use the project_id in context
if project_id is None:
project_id = context.project_id
# Get the applicable quotas
quotas = self._get_quotas(context, resources, values.keys(),
has_sync=False)
has_sync=False, project_id=project_id)
# Check the quotas and construct a list of the resources that
# would be put over limit by the desired values
overs = [key for key, val in values.items()
@ -248,7 +258,8 @@ class DbQuotaDriver(object):
raise exception.OverQuota(overs=sorted(overs), quotas=quotas,
usages={})
def reserve(self, context, resources, deltas, expire=None):
def reserve(self, context, resources, deltas, expire=None,
project_id=None):
"""Check quotas and reserve resources.
For counting quotas--those quotas for which there is a usage
@ -278,6 +289,9 @@ class DbQuotaDriver(object):
default expiration time set by
--default-reservation-expire will be used (this
value will be treated as a number of seconds).
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
# Set up the reservation expiration
@ -290,12 +304,16 @@ class DbQuotaDriver(object):
if not isinstance(expire, datetime.datetime):
raise exception.InvalidReservationExpiration(expire=expire)
# If project_id is None, then we use the project_id in context
if project_id is None:
project_id = context.project_id
# Get the applicable quotas.
# NOTE(Vek): We're not worried about races at this point.
# Yes, the admin may be in the process of reducing
# quotas, but that's a pretty rare thing.
quotas = self._get_quotas(context, resources, deltas.keys(),
has_sync=True)
has_sync=True, project_id=project_id)
# NOTE(Vek): Most of the work here has to be done in the DB
# API, because we have to do it in a transaction,
@ -303,27 +321,40 @@ class DbQuotaDriver(object):
# session isn't available outside the DBAPI, we
# have to do the work there.
return db.quota_reserve(context, resources, quotas, deltas, expire,
FLAGS.until_refresh, FLAGS.max_age)
FLAGS.until_refresh, FLAGS.max_age,
project_id=project_id)
def commit(self, context, reservations):
def commit(self, context, reservations, project_id=None):
"""Commit reservations.
:param context: The request context, for access checks.
:param reservations: A list of the reservation UUIDs, as
returned by the reserve() method.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
# If project_id is None, then we use the project_id in context
if project_id is None:
project_id = context.project_id
db.reservation_commit(context, reservations)
db.reservation_commit(context, reservations, project_id=project_id)
def rollback(self, context, reservations):
def rollback(self, context, reservations, project_id=None):
"""Roll back reservations.
:param context: The request context, for access checks.
:param reservations: A list of the reservation UUIDs, as
returned by the reserve() method.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
# If project_id is None, then we use the project_id in context
if project_id is None:
project_id = context.project_id
db.reservation_rollback(context, reservations)
db.reservation_rollback(context, reservations, project_id=project_id)
def destroy_all_by_project(self, context, project_id):
"""
@ -603,7 +634,7 @@ class QuotaEngine(object):
return res.count(context, *args, **kwargs)
def limit_check(self, context, **values):
def limit_check(self, context, project_id=None, **values):
"""Check simple quota limits.
For limits--those quotas for which there is no usage
@ -623,11 +654,15 @@ class QuotaEngine(object):
nothing.
:param context: The request context, for access checks.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
return self._driver.limit_check(context, self._resources, values)
return self._driver.limit_check(context, self._resources, values,
project_id=project_id)
def reserve(self, context, expire=None, **deltas):
def reserve(self, context, expire=None, project_id=None, **deltas):
"""Check quotas and reserve resources.
For counting quotas--those quotas for which there is a usage
@ -657,25 +692,32 @@ class QuotaEngine(object):
default expiration time set by
--default-reservation-expire will be used (this
value will be treated as a number of seconds).
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
reservations = self._driver.reserve(context, self._resources, deltas,
expire=expire)
expire=expire,
project_id=project_id)
LOG.debug(_("Created reservations %(reservations)s") % locals())
return reservations
def commit(self, context, reservations):
def commit(self, context, reservations, project_id=None):
"""Commit reservations.
:param context: The request context, for access checks.
:param reservations: A list of the reservation UUIDs, as
returned by the reserve() method.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
try:
self._driver.commit(context, reservations)
self._driver.commit(context, reservations, project_id=project_id)
except Exception:
# NOTE(Vek): Ignoring exceptions here is safe, because the
# usage resynchronization and the reservation expiration
@ -684,16 +726,19 @@ class QuotaEngine(object):
LOG.exception(_("Failed to commit reservations "
"%(reservations)s") % locals())
def rollback(self, context, reservations):
def rollback(self, context, reservations, project_id=None):
"""Roll back reservations.
:param context: The request context, for access checks.
:param reservations: A list of the reservation UUIDs, as
returned by the reserve() method.
:param project_id: Specify the project_id if current context
is admin and admin wants to impact on
common user's tenant.
"""
try:
self._driver.rollback(context, reservations)
self._driver.rollback(context, reservations, project_id=project_id)
except Exception:
# NOTE(Vek): Ignoring exceptions here is safe, because the
# usage resynchronization and the reservation expiration

@ -193,18 +193,21 @@ class FakeDriver(object):
project_id, quota_class, defaults, usages))
return resources
def limit_check(self, context, resources, values):
self.called.append(('limit_check', context, resources, values))
def limit_check(self, context, resources, values, project_id=None):
self.called.append(('limit_check', context, resources,
values, project_id))
def reserve(self, context, resources, deltas, expire=None):
self.called.append(('reserve', context, resources, deltas, expire))
def reserve(self, context, resources, deltas, expire=None,
project_id=None):
self.called.append(('reserve', context, resources, deltas,
expire, project_id))
return self.reservations
def commit(self, context, reservations):
self.called.append(('commit', context, reservations))
def commit(self, context, reservations, project_id=None):
self.called.append(('commit', context, reservations, project_id))
def rollback(self, context, reservations):
self.called.append(('rollback', context, reservations))
def rollback(self, context, reservations, project_id=None):
self.called.append(('rollback', context, reservations, project_id))
def destroy_all_by_project(self, context, project_id):
self.called.append(('destroy_all_by_project', context, project_id))
@ -518,7 +521,8 @@ class QuotaEngineTestCase(test.TestCase):
test_resource1=4,
test_resource2=3,
test_resource3=2,
test_resource4=1,)), ])
test_resource4=1,),
None), ])
def test_reserve(self):
context = FakeContext(None, None)
@ -533,6 +537,9 @@ class QuotaEngineTestCase(test.TestCase):
result2 = quota_obj.reserve(context, expire=3600,
test_resource1=1, test_resource2=2,
test_resource3=3, test_resource4=4)
result3 = quota_obj.reserve(context, project_id='fake_project',
test_resource1=1, test_resource2=2,
test_resource3=3, test_resource4=4)
self.assertEqual(driver.called, [
('reserve',
@ -543,6 +550,7 @@ class QuotaEngineTestCase(test.TestCase):
test_resource2=3,
test_resource3=2,
test_resource4=1, ),
None,
None),
('reserve',
context,
@ -552,7 +560,18 @@ class QuotaEngineTestCase(test.TestCase):
test_resource2=2,
test_resource3=3,
test_resource4=4, ),
3600), ])
3600,
None),
('reserve',
context,
quota_obj._resources,
dict(
test_resource1=1,
test_resource2=2,
test_resource3=3,
test_resource4=4, ),
None,
'fake_project'), ])
self.assertEqual(result1, ['resv-01',
'resv-02',
'resv-03',
@ -561,6 +580,10 @@ class QuotaEngineTestCase(test.TestCase):
'resv-02',
'resv-03',
'resv-04', ])
self.assertEqual(result3, ['resv-01',
'resv-02',
'resv-03',
'resv-04', ])
def test_commit(self):
context = FakeContext(None, None)
@ -573,7 +596,8 @@ class QuotaEngineTestCase(test.TestCase):
context,
['resv-01',
'resv-02',
'resv-03']), ])
'resv-03'],
None), ])
def test_rollback(self):
context = FakeContext(None, None)
@ -586,7 +610,8 @@ class QuotaEngineTestCase(test.TestCase):
context,
['resv-01',
'resv-02',
'resv-03']), ])
'resv-03'],
None), ])
def test_destroy_all_by_project(self):
context = FakeContext(None, None)
@ -838,7 +863,7 @@ class DbQuotaDriverTestCase(test.TestCase):
def _stub_quota_reserve(self):
def fake_quota_reserve(context, resources, quotas, deltas, expire,
until_refresh, max_age):
until_refresh, max_age, project_id=None):
self.calls.append(('quota_reserve', expire, until_refresh,
max_age))
return ['resv-1', 'resv-2', 'resv-3']
@ -996,7 +1021,7 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
def fake_get_session():
return FakeSession()
def fake_get_quota_usages(context, session):
def fake_get_quota_usages(context, session, project_id):
return self.usages.copy()
def fake_quota_usage_create(context, project_id, resource, in_use,

@ -96,13 +96,13 @@ class VolumeTestCase(test.TestCase):
def test_create_delete_volume(self):
"""Test volume can be created and deleted."""
# Need to stub out reserve, commit, and rollback
def fake_reserve(context, expire=None, **deltas):
def fake_reserve(context, expire=None, project_id=None, **deltas):
return ["RESERVATION"]
def fake_commit(context, reservations):
def fake_commit(context, reservations, project_id=None):
pass
def fake_rollback(context, reservations):
def fake_rollback(context, reservations, project_id=None):
pass
self.stubs.Set(QUOTAS, "reserve", fake_reserve)
@ -160,13 +160,13 @@ class VolumeTestCase(test.TestCase):
def test_create_volume_with_volume_type(self):
"""Test volume creation with default volume type."""
def fake_reserve(context, expire=None, **deltas):
def fake_reserve(context, expire=None, project_id=None, **deltas):
return ["RESERVATION"]
def fake_commit(context, reservations):
def fake_commit(context, reservations, project_id=None):
pass
def fake_rollback(context, reservations):
def fake_rollback(context, reservations, project_id=None):
pass
self.stubs.Set(QUOTAS, "reserve", fake_reserve)
@ -746,13 +746,13 @@ class VolumeTestCase(test.TestCase):
'name', 'description', image_id=1)
def _do_test_create_volume_with_size(self, size):
def fake_reserve(context, expire=None, **deltas):
def fake_reserve(context, expire=None, project_id=None, **deltas):
return ["RESERVATION"]
def fake_commit(context, reservations):
def fake_commit(context, reservations, project_id=None):
pass
def fake_rollback(context, reservations):
def fake_rollback(context, reservations, project_id=None):
pass
self.stubs.Set(QUOTAS, "reserve", fake_reserve)
@ -776,13 +776,13 @@ class VolumeTestCase(test.TestCase):
self._do_test_create_volume_with_size('2')
def test_create_volume_with_bad_size(self):
def fake_reserve(context, expire=None, **deltas):
def fake_reserve(context, expire=None, project_id=None, **deltas):
return ["RESERVATION"]
def fake_commit(context, reservations):
def fake_commit(context, reservations, project_id=None):
pass
def fake_rollback(context, reservations):
def fake_rollback(context, reservations, project_id=None):
pass
self.stubs.Set(QUOTAS, "reserve", fake_reserve)

@ -279,12 +279,19 @@ class API(base.Base):
@wrap_check_policy
def delete(self, context, volume, force=False):
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']:
# NOTE(vish): scheduling failed, so delete it
# Note(zhiteng): update volume quota reservation
try:
reservations = QUOTAS.reserve(context, volumes=-1,
reservations = QUOTAS.reserve(context,
project_id=project_id,
volumes=-1,
gigabytes=-volume['size'])
except Exception:
reservations = None
@ -292,7 +299,7 @@ class API(base.Base):
self.db.volume_destroy(context.elevated(), volume_id)
if reservations:
QUOTAS.commit(context, reservations)
QUOTAS.commit(context, reservations, project_id=project_id)
return
if not force and volume['status'] not in ["available", "error",
"error_restoring"]:

@ -393,6 +393,12 @@ class VolumeManager(manager.SchedulerDependentManager):
"""Deletes and unexports volume."""
context = context.elevated()
volume_ref = self.db.volume_get(context, volume_id)
if context.project_id != volume_ref['project_id']:
project_id = volume_ref['project_id']
else:
project_id = context.project_id
LOG.info(_("volume %s: deleting"), volume_ref['name'])
if volume_ref['attach_status'] == "attached":
# Volume is still attached, need to detach first
@ -422,7 +428,9 @@ class VolumeManager(manager.SchedulerDependentManager):
# Get reservations
try:
reservations = QUOTAS.reserve(context, volumes=-1,
reservations = QUOTAS.reserve(context,
project_id=project_id,
volumes=-1,
gigabytes=-volume_ref['size'])
except Exception:
reservations = None
@ -435,7 +443,7 @@ class VolumeManager(manager.SchedulerDependentManager):
# Commit the reservations
if reservations:
QUOTAS.commit(context, reservations)
QUOTAS.commit(context, reservations, project_id=project_id)
return True
@ -474,6 +482,11 @@ class VolumeManager(manager.SchedulerDependentManager):
snapshot_ref = self.db.snapshot_get(context, snapshot_id)
LOG.info(_("snapshot %s: deleting"), snapshot_ref['name'])
if context.project_id != snapshot_ref['project_id']:
project_id = snapshot_ref['project_id']
else:
project_id = context.project_id
try:
LOG.debug(_("snapshot %s: deleting"), snapshot_ref['name'])
self.driver.delete_snapshot(snapshot_ref)
@ -492,10 +505,13 @@ class VolumeManager(manager.SchedulerDependentManager):
# Get reservations
try:
if CONF.no_snapshot_gb_quota:
reservations = QUOTAS.reserve(context, snapshots=-1)
reservations = QUOTAS.reserve(context,
project_id=project_id,
snapshots=-1)
else:
reservations = QUOTAS.reserve(
context,
project_id=project_id,
snapshots=-1,
gigabytes=-snapshot_ref['volume_size'])
except Exception:
@ -507,7 +523,7 @@ class VolumeManager(manager.SchedulerDependentManager):
# Commit the reservations
if reservations:
QUOTAS.commit(context, reservations)
QUOTAS.commit(context, reservations, project_id=project_id)
return True
def attach_volume(self, context, volume_id, instance_uuid, mountpoint):