openstack
/
cinder
Code Issues Proposed changes

Prevent quota and reservations to go into negative 

Our current quota usage code doesn't prevent our usage or reservations
to go into negative, though it warns at reservation time.

This patch makes sure that we don't go into negative, setting it to 0 in
case it would be negative.

Change-Id: I487157d7092fa4cee1cd72cf01ce7031c94f553f
This commit is contained in:
Gorka Eguileor 2021-03-15 17:00:31 +01:00
parent cdd60605f4
commit 935ddf7c72
2 changed files with 105 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
cinder/db/sqlalchemy/api.py
View File

@ -1289,8 +1289,9 @@ def quota_reserve(context, resources, quotas, deltas, expire,
usages[resource].reserved += delta
if unders:
LOG.warning("Change will make usage less than 0 for the following "
"resources: %s", unders)
LOG.warning("Reservation would make usage less than 0 for the "
"following resources, so on commit they will be "
"limited to prevent going below 0: %s", unders)
if overs:
usages = {k: dict(in_use=v.in_use, reserved=v.reserved)
for k, v in usages.items()}
@ -1345,9 +1346,14 @@ def reservation_commit(context, reservations, project_id=None):
for reservation in _quota_reservations(session, context, reservations):
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
usage.in_use += reservation.delta
delta = reservation.delta
if delta >= 0:
usage.reserved -= min(delta, usage.reserved)
# For negative deltas make sure we never go into negative usage
elif -delta > usage.in_use:
delta = -usage.in_use
usage.in_use += delta
reservation.delete(session=session)
@ -1365,7 +1371,7 @@ def reservation_rollback(context, reservations, project_id=None):
for reservation in _quota_reservations(session, context, reservations):
usage = usages[reservation.usage_id]
if reservation.delta >= 0:
usage.reserved -= reservation.delta
usage.reserved -= min(reservation.delta, usage.reserved)
reservation.delete(session=session)
@ -1434,7 +1440,8 @@ def reservation_expire(context):
if results:
for reservation in results:
if reservation.delta >= 0:
reservation.usage.reserved -= reservation.delta
reservation.usage.reserved -= min(
reservation.delta, reservation.usage.reserved)
reservation.usage.save(session=session)
reservation.delete(session=session)

95
cinder/tests/unit/test_db_api.py
View File

@ -45,7 +45,7 @@ ONE_HUNDREDS = 100
UTC_NOW = timeutils.utcnow()
def _quota_reserve(context, project_id):
def _quota_reserve(context, project_id, **resource_dict):
"""Create sample Quota, QuotaUsage and Reservation objects.
There is no method db.quota_usage_create(), so we have to use
@ -58,15 +58,19 @@ def _quota_reserve(context, project_id):
def sync(elevated, project_id, session):
return {resource: usage}
return sync
if not resource_dict:
resource_dict = {'volumes': 1, 'gigabytes': 2}
quotas = {}
resources = {}
deltas = {}
for i, resource in enumerate(('volumes', 'gigabytes')):
quota_obj = db.quota_create(context, project_id, resource, i + 1)
for resource, value in resource_dict.items():
quota_obj = db.quota_create(context, project_id, resource, value)
quotas[resource] = quota_obj.hard_limit
resources[resource] = quota.ReservableResource(resource,
'_sync_%s' % resource)
deltas[resource] = i + 1
deltas[resource] = value
return db.quota_reserve(
context, resources, quotas, deltas,
datetime.datetime.utcnow(), until_refresh=None,
@ -2530,6 +2534,47 @@ class DBAPIReservationTestCase(BaseTest):
self.ctxt,
'project1'))
def test_reservation_commit_negative_reservation(self):
"""Verify we can't make reservations negative on commit."""
project = 'project1'
reservations = _quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage.reserved -= 1
vol_usage.save(session=session)
# When committing 2 volumes from reserved to used reserved should not
# go from 1 to -1 but from 1 to 0, but in-use should still increase by
# 2
db.reservation_commit(self.ctxt, reservations, project)
expected = {'project_id': project,
'volumes': {'reserved': 0, 'in_use': 2}}
self.assertEqual(expected,
db.quota_usage_get_all_by_project(self.ctxt, project))
def test_reservation_commit_negative_in_use(self):
"""Verify we can't make in-use negative on commit."""
project = 'project1'
reservations = _quota_reserve(self.ctxt, project, volumes=-2)
# Force a smaller in_use than the one the reservation will decrease
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, 'project1', 'volumes')
vol_usage.in_use = 1
vol_usage.save(session=session)
# When committing -2 volumes from reserved to in-use they should not
# make in-use go from 1 to -1, but from 1 to 0
db.reservation_commit(self.ctxt, reservations, project)
expected = {'project_id': project,
'volumes': {'reserved': 0, 'in_use': 0}}
self.assertEqual(expected,
db.quota_usage_get_all_by_project(self.ctxt, project))
def test_reservation_rollback(self):
reservations = _quota_reserve(self.ctxt, 'project1')
expected = {'project_id': 'project1',
@ -2550,6 +2595,27 @@ class DBAPIReservationTestCase(BaseTest):
self.ctxt,
'project1'))
def test_reservation_rollback_negative(self):
"""Verify we can't make reservations negative on rollback."""
project = 'project1'
reservations = _quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage.reserved -= 1
vol_usage.save(session=session)
# When rolling back 2 volumes from reserved when there's only 1 in the
# quota usage's reserved field, reserved should not go from 1 to -1
# but from 1 to 0
db.reservation_rollback(self.ctxt, reservations, project)
expected = {'project_id': project,
'volumes': {'reserved': 0, 'in_use': 0}}
self.assertEqual(expected,
db.quota_usage_get_all_by_project(self.ctxt, project))
def test_reservation_expire(self):
self.values['expire'] = datetime.datetime.utcnow() + \
datetime.timedelta(days=1)
@ -2564,6 +2630,27 @@ class DBAPIReservationTestCase(BaseTest):
self.ctxt,
'project1'))
def test_reservation_expire_negative(self):
"""Verify we can't make reservation negative on expiration."""
project = 'project1'
_quota_reserve(self.ctxt, project, volumes=2)
# Force a smaller reserved value in quota_usages table
session = sqlalchemy_api.get_session()
with session.begin():
vol_usage = db.quota_usage_get(self.ctxt, project, 'volumes')
vol_usage.reserved -= 1
vol_usage.save(session=session)
# When expiring 2 volumes from reserved when there's only 1 in the
# quota usage's reserved field, reserved should not go from 1 to -1
# but from 1 to 0
db.reservation_expire(self.ctxt)
expected = {'project_id': project,
'volumes': {'reserved': 0, 'in_use': 0}}
self.assertEqual(expected,
db.quota_usage_get_all_by_project(self.ctxt, project))
@mock.patch('time.sleep', mock.Mock())
def test_quota_reserve_create_usages_race(self):
"""Test we retry when there is a race in creation."""