Filter out deleted images from storage usage
All database API's currently include deleted images in the calc of
storage usage. This is not an issue when deleted images don't have
locations. However, there are cases where a deleted image has deleted
locations as well and that causes the current algorithm to count those
locations as if they were allocating space.
Besides this bug, it makes sense to not load deleted / killed /
pending_delete images from the database if we're actually not
considering them as valid images.
The patch also filters out deleted locations.
NOTE: In the case of locations, it was not possible to add a test for
the deleted locations because it requires some changes that are not
worth in this patch. In order to mark a location as deleted, it's
necessary to go through the API and use a PATCH operation. Since this is
a database test, it doesn't make much sense to add API calls to it.
Calling the image_destroy function with an empty location list will
remove all the locations which won't help testing that specific case.
I'll work on a better solution for that in a follow-up patch.
DocImpact:
The patch now excludes deleted images from the count, this fixes a
bug but changes the existing behaviour.
The patch excludes images in pending_delete from the count, although
the space hasn't be freed yet. This may cause the quota to be
exceeded without raising an error until the image is finally deleted
from the store.
Conflicts:
glance/tests/functional/db/test_sqlalchemy.py
Closes-Bug: #1261738
(cherry picked from commit b35728019e
)
Change-Id: I82f08a8f522c81541e4f77597c2ba0aeb68556ce
This commit is contained in:
parent
5cd7a22b44
commit
4e26df9c92
|
@ -513,6 +513,12 @@ def image_destroy(context, image_id):
|
|||
DATA['images'][image_id]['deleted'] = True
|
||||
DATA['images'][image_id]['deleted_at'] = timeutils.utcnow()
|
||||
|
||||
# NOTE(flaper87): Move the image to one of the deleted statuses
|
||||
# if it hasn't been done yet.
|
||||
if (DATA['images'][image_id]['status'] not in
|
||||
['deleted', 'pending_delete']):
|
||||
DATA['images'][image_id]['status'] = 'deleted'
|
||||
|
||||
_image_locations_set(image_id, [])
|
||||
|
||||
for prop in DATA['images'][image_id]['properties']:
|
||||
|
@ -654,6 +660,11 @@ def user_get_storage_usage(context, owner_id, image_id=None, session=None):
|
|||
images = image_get_all(context, filters={'owner': owner_id})
|
||||
total = 0
|
||||
for image in images:
|
||||
if image['status'] in ['killed', 'pending_delete', 'deleted']:
|
||||
continue
|
||||
|
||||
if image['id'] != image_id:
|
||||
total = total + (image['size'] * len(image['locations']))
|
||||
locations = [l for l in image['locations']
|
||||
if not l.get('deleted', False)]
|
||||
total += (image['size'] * len(locations))
|
||||
return total
|
||||
|
|
|
@ -694,8 +694,15 @@ def _image_get_disk_usage_by_owner(owner, session, image_id=None):
|
|||
if image_id is not None:
|
||||
query = query.filter(models.Image.id != image_id)
|
||||
query = query.filter(models.Image.size > 0)
|
||||
query = query.filter(~models.Image.status.in_(['killed',
|
||||
'pending_delete',
|
||||
'deleted']))
|
||||
images = query.all()
|
||||
total = sum([i.size * len(i.locations) for i in images])
|
||||
|
||||
total = 0
|
||||
for i in images:
|
||||
locations = [l for l in i.locations if not l['deleted']]
|
||||
total += (i.size * len(locations))
|
||||
return total
|
||||
|
||||
|
||||
|
|
|
@ -1097,7 +1097,8 @@ class DriverQuotaTests(test_utils.BaseTestCase):
|
|||
super(DriverQuotaTests, self).setUp()
|
||||
self.owner_id1 = uuidutils.generate_uuid()
|
||||
self.context1 = context.RequestContext(
|
||||
is_admin=False, auth_tok='user:user:user', user=self.owner_id1)
|
||||
is_admin=False, user=self.owner_id1, tenant=self.owner_id1,
|
||||
auth_tok='%s:%s:user' % (self.owner_id1, self.owner_id1))
|
||||
self.db_api = db_tests.get_db(self.config)
|
||||
db_tests.reset_db(self.db_api)
|
||||
self.addCleanup(timeutils.clear_time_override)
|
||||
|
@ -1163,6 +1164,29 @@ class DriverQuotaTests(test_utils.BaseTestCase):
|
|||
x = self.db_api.user_get_storage_usage(self.context1, self.owner_id1)
|
||||
self.assertEqual(total, x)
|
||||
|
||||
def test_storage_quota_deleted_image(self):
|
||||
# NOTE(flaper87): This needs to be tested for
|
||||
# soft deleted images as well. Currently there's no
|
||||
# good way to delete locations.
|
||||
dt1 = timeutils.utcnow()
|
||||
sz = 53
|
||||
new_fixture_dict = {'id': 'SOMEID', 'created_at': dt1,
|
||||
'updated_at': dt1, 'size': sz,
|
||||
'owner': self.owner_id1}
|
||||
new_fixture = build_image_fixture(**new_fixture_dict)
|
||||
new_fixture['locations'].append({'url': 'file:///some/path/file',
|
||||
'metadata': {}})
|
||||
self.db_api.image_create(self.context1, new_fixture)
|
||||
|
||||
total = reduce(lambda x, y: x + y,
|
||||
[f['size'] for f in self.owner1_fixtures])
|
||||
x = self.db_api.user_get_storage_usage(self.context1, self.owner_id1)
|
||||
self.assertEqual(total + (sz * 2), x)
|
||||
|
||||
self.db_api.image_destroy(self.context1, 'SOMEID')
|
||||
x = self.db_api.user_get_storage_usage(self.context1, self.owner_id1)
|
||||
self.assertEqual(total, x)
|
||||
|
||||
|
||||
class TestVisibility(test_utils.BaseTestCase):
|
||||
def setUp(self):
|
||||
|
|
|
@ -96,3 +96,11 @@ class TestSqlAlchemyDBDataIntegrity(base.TestDriver):
|
|||
fake_paginate_query)
|
||||
images = self.db_api.image_get_all(self.context,
|
||||
sort_key='name')
|
||||
|
||||
|
||||
class TestSqlAlchemyQuota(base.DriverQuotaTests):
|
||||
|
||||
def setUp(self):
|
||||
db_tests.load(get_db, reset_db)
|
||||
super(TestSqlAlchemyQuota, self).setUp()
|
||||
self.addCleanup(db_tests.reset)
|
||||
|
|
Loading…
Reference in New Issue