Openstack Glance: user_total_quota calculated incorrectly

[Tzach Shefi]

Description of problem: Bug in quota calculation, if an image upload fails due to quota limit, the failed image size is still added to total storage sum figure! Thus future images may fail to upload even if it looks as quota hasn’t been reached yet.

Version-Release number of selected component (if applicable):
RHEL 6.5

python-glanceclient-0.12.0-1.el6ost.noarch
openstack-glance-2013.2-5.el6ost.noarch
python-glance-2013.2-5.el6ost.noarch

How reproducible:

Steps to Reproduce:
1. vim /etc/glance/glance-api.conf user_storage_quota = 250mb (in byets)
2. service glance-api restart
3. Upload test small image - would be ok
4. Upload large image say 4Giga, - should fail with "Error unable to create new image"

5. Try to upload another small file say 49MB.

Actual results:

If the large i,age file or sum of failed uploaded images are more than the quota, any image size will fail to upload.

Expected results:

I should be able to upload as long as the sum of all my images is less than configured qouta.

Additional info:

Mysql show databases;
connect glance;
SELECT * FROM images;

Noticed all the images i tired, initial successful uploaded image status=”active”, images that i deleted status=”deleted”, images that failed to upload due to quota status=”killed”

I than calculated the sum of all the “killed” images.
Set a new quota of the above calculated value + 100MB, restarted glance-api service.
Only than i was able to upload another image of 49MB.

When i set a lower quota value (below the calculated sum of all the killed images) wasn’t able to upload any image.

Images of status killed, which fail upload for any reason, should not be added to total storage sum calcualtion or quota.

[Feilong Wang (flwang)]

I tested it with my local env and can't reproduce it. And after reviewed the code, I don't think the 'killed' image size will be counted. See https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/api.py#L716

In other words, the size will multiply the number of locations. However, the locations number of 'killed' image is 0. So it won't be counted.

Please feel free reopen this bug if I missed anything. Thanks.

[Zhi Yan Liu (lzy-dev)]

I think we need do this to fix this defect, i verified it in my dev local.

http://paste.openstack.org/show/55299/

[Flavio Percoco (flaper87)]

I reproduced it. The issue is that the query is not filtering deleted images out. I'll take it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/62897

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/63455

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/62897
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=b35728019e0eb89c213eed7bc35a1f062c99dcca
Submitter: Jenkins
Branch: master

commit b35728019e0eb89c213eed7bc35a1f062c99dcca
Author: Flavio Percoco <email address hidden>
Date: Wed Dec 18 15:18:17 2013 +0100

    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

    Change-Id: I82f08a8f522c81541e4f77597c2ba0aeb68556ce
    Closes-Bug: #1261738

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/63455
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=4e26df9c922d96473fdd27782af0fed93f0a79de
Submitter: Jenkins
Branch: stable/havana

commit 4e26df9c922d96473fdd27782af0fed93f0a79de
Author: Flavio Percoco <email address hidden>
Date: Wed Dec 18 15:18:17 2013 +0100

    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 b35728019e0eb89c213eed7bc35a1f062c99dcca)
    Change-Id: I82f08a8f522c81541e4f77597c2ba0aeb68556ce