openstack/manila
Code Issues Proposed changes

Update quota of proper user on resource delete

Browse Source 
When deleting a resource (share, snapshot, share-network) the quota
usage of the current user gets updated, which might not be the same
user, who created that resource. That means the quota usage will never
be reduced for initial user.
This fix adds the resources user_id to the quota update statements, to
update the quota_usage for the user, that created this resource,
irregardless who deletes it.

Change-Id: Iefe8f4d0e7c526e3ed94c1994ba62f1a2a929ba2
Closes-Bug: #1542598
This commit is contained in:
Tom Patzig 2016-03-08 16:55:04 +01:00
parent 83169e0f7d
commit 8d3e72f8b2
7 changed files with 86 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
manila/api/v1/share_networks.py
View File

@ -97,13 +97,14 @@ class ShareNetworkController(wsgi.Controller):
try:
reservations = QUOTAS.reserve(
context, project_id=share_network['project_id'],
share_networks=-1)
share_networks=-1, user_id=share_network['user_id'])
except Exception:
LOG.exception(_LE("Failed to update usages deleting "
"share-network."))
else:
QUOTAS.commit(context, reservations,
project_id=share_network['project_id'])
project_id=share_network['project_id'],
user_id=share_network['user_id'])
return webob.Response(status_int=202)
def _get_share_networks(self, req, is_detail=True):
@ -247,6 +248,7 @@ class ShareNetworkController(wsgi.Controller):
values = body[RESOURCE_NAME]
values['project_id'] = context.project_id
values['user_id'] = context.user_id
self._verify_no_mutually_exclusive_data(values)
try:

20
manila/quota.py
View File

@ -301,7 +301,8 @@ class DbQuotaDriver(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
# Filter resources
@ -358,7 +359,8 @@ class DbQuotaDriver(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
# Ensure no value is less than zero
@ -425,7 +427,8 @@ class DbQuotaDriver(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
# Set up the reservation expiration
@ -441,7 +444,7 @@ class DbQuotaDriver(object):
# If project_id is None, then we use the project_id in context
if project_id is None:
project_id = context.project_id
# If user_id is None, then we use the project_id in context
# If user_id is None, then we use the user_id in context
if user_id is None:
user_id = context.user_id
@ -476,7 +479,8 @@ class DbQuotaDriver(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
# If project_id is None, then we use the project_id in context
if project_id is None:
@ -499,7 +503,8 @@ class DbQuotaDriver(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
# If project_id is None, then we use the project_id in context
if project_id is None:
@ -912,7 +917,8 @@ class QuotaEngine(object):
common user's tenant.
:param user_id: Specify the user_id if current context
is admin and admin wants to impact on
common user.
common user. (Special case: user operates on
resource, owned/created by different user)
"""
return self._driver.limit_check(context, self._resources, values,

10
manila/share/api.py
View File

@ -605,10 +605,13 @@ class API(base.Base):
self._check_is_share_busy(share)
try:
# we give the user_id of the share, to update the quota usage
# for the user, who created the share
reservations = QUOTAS.reserve(context,
project_id=project_id,
shares=-1,
gigabytes=-share['size'])
gigabytes=-share['size'],
user_id=share['user_id'])
except Exception as e:
reservations = None
LOG.exception(
@ -623,7 +626,10 @@ class API(base.Base):
self.db.share_instance_delete(context, share_instance['id'])
if reservations:
QUOTAS.commit(context, reservations, project_id=project_id)
# we give the user_id of the share, to update the quota usage
# for the user, who created the share
QUOTAS.commit(context, reservations, project_id=project_id,
user_id=share['user_id'])
def delete_instance(self, context, share_instance, force=False):
policy.check_policy(context, 'share', 'delete')

6
manila/share/manager.py
View File

@ -1808,13 +1808,15 @@ class ShareManager(manager.SchedulerDependentManager):
try:
reservations = QUOTAS.reserve(
context, project_id=project_id, snapshots=-1,
snapshot_gigabytes=-snapshot_ref['size'])
snapshot_gigabytes=-snapshot_ref['size'],
user_id=snapshot_ref['user_id'])
except Exception:
reservations = None
LOG.exception(_LE("Failed to update usages deleting snapshot"))
if reservations:
QUOTAS.commit(context, reservations, project_id=project_id)
QUOTAS.commit(context, reservations, project_id=project_id,
user_id=snapshot_ref['user_id'])
@add_hooks
@utils.require_driver_initialized

9
manila/tests/api/v1/test_share_networks.py
View File

@ -238,6 +238,8 @@ class ShareNetworkAPITest(test.TestCase):
def test_quota_delete_reservation_failed(self):
share_nw = fake_share_network.copy()
share_nw['share_servers'] = ['foo', 'bar']
share_nw['user_id'] = 'fake_user_id'
self.mock_object(db_api, 'share_network_get',
mock.Mock(return_value=share_nw))
self.mock_object(db_api, 'share_instances_get_all_by_share_network',
@ -262,7 +264,12 @@ class ShareNetworkAPITest(test.TestCase):
mock.call(self.req.environ['manila.context'], 'bar')])
db_api.share_network_delete.assert_called_once_with(
self.req.environ['manila.context'], share_nw['id'])
self.assertTrue(share_networks.QUOTAS.reserve.called)
share_networks.QUOTAS.reserve.assert_called_once_with(
self.req.environ['manila.context'],
project_id=share_nw['project_id'],
share_networks=-1,
user_id=share_nw['user_id']
)
self.assertFalse(share_networks.QUOTAS.commit.called)
def test_delete_in_use_by_share(self):

27
manila/tests/share/test_api.py
View File

@ -1082,6 +1082,30 @@ class ShareAPITestCase(test.TestCase):
db_api.share_snapshot_get_all_for_share.assert_called_once_with(
utils.IsAMatcher(context.RequestContext), share['id'])
def test_delete_quota_with_different_user(self):
share = self._setup_delete_mocks(constants.STATUS_AVAILABLE)
diff_user_context = context.RequestContext(
user_id='fake2',
project_id='fake',
is_admin=False
)
self.api.delete(diff_user_context, share)
quota.QUOTAS.reserve.assert_called_once_with(
diff_user_context,
project_id=share['project_id'],
shares=-1,
gigabytes=-share['size'],
user_id=share['user_id']
)
quota.QUOTAS.commit.assert_called_once_with(
diff_user_context,
mock.ANY,
project_id=share['project_id'],
user_id=share['user_id']
)
def test_delete_wrong_status(self):
share = fake_share('fakeid')
self.mock_object(db_api, 'share_get', mock.Mock(return_value=share))
@ -1148,7 +1172,8 @@ class ShareAPITestCase(test.TestCase):
self.context,
project_id=share['project_id'],
shares=-1,
gigabytes=-share['size']
gigabytes=-share['size'],
user_id=share['user_id']
)
self.assertFalse(quota.QUOTAS.commit.called)

23
manila/tests/share/test_manager.py
View File

@ -1308,6 +1308,29 @@ class ShareManagerTestCase(test.TestCase):
utils.IsAMatcher(models.ShareSnapshotInstance),
share_server=None)
def test_delete_snapshot_quota_error(self):
share = db_utils.create_share()
share_id = share['id']
snapshot = db_utils.create_snapshot(share_id=share_id)
snapshot_id = snapshot['id']
snapshot = db_utils.create_snapshot(
with_share=True, status=constants.STATUS_AVAILABLE)
self.mock_object(quota.QUOTAS, 'reserve',
mock.Mock(side_effect=exception.QuotaError('fake')))
self.mock_object(quota.QUOTAS, 'commit')
self.share_manager.delete_snapshot(self.context, snapshot_id)
quota.QUOTAS.reserve.assert_called_once_with(
mock.ANY,
project_id=six.text_type(snapshot['project_id']),
snapshots=-1,
snapshot_gigabytes=-snapshot['size'],
user_id=six.text_type(snapshot['user_id'])
)
self.assertFalse(quota.QUOTAS.commit.called)
def test_delete_share_instance_if_busy(self):
"""Test snapshot could not be deleted if busy."""