From 87a9338466406bc0575bc5fc00ae9cc2ebdb342e Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Mon, 15 Mar 2021 15:12:13 +0100 Subject: [PATCH] Fix automatic quota sync for temporary volumes When using the automatic quota refresh via `until_refresh` and `max_age` configuration options the calculated quota usage by the refresh will not be correct if there are temporary volumes (such as those created from snapshots for backups). Normal quota usage calculation does not work like this, as it has admin metadata stating that those are temporary volumes and are not used to increase/decrease quota usage. This patch fixes this by modifying existing filter that ignores migrating volumes so it can also ignore temporary volumes based on their admin metadata. Closes-Bug: #1919161 Change-Id: I0fd89028684c1406f427f4c97b120ece5cf2780b (cherry picked from commit 8f03b26f50dafd161d5de5a6964fc9803177c1ec) --- cinder/db/sqlalchemy/api.py | 14 ++++++++++-- cinder/tests/unit/test_db_api.py | 22 +++++++++++++++++++ ...quota-sync-temporary-b4103ebc2c484c89.yaml | 8 +++++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/quota-sync-temporary-b4103ebc2c484c89.yaml diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index d721afa80dd..082c8274c1b 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1593,10 +1593,20 @@ def _volume_data_get_for_project(context, project_id, volume_type_id=None, # When calling the method for quotas we don't count volumes that are the # destination of a migration since they were not accounted for quotas or # reservations in the first place. + # Also skip temporary volumes that have 'temporary' admin_metadata key set + # to True. if skip_internal: + admin_model = models.VolumeAdminMetadata query = query.filter( - or_(model.migration_status.is_(None), - ~model.migration_status.startswith('target:'))) + and_(or_(model.migration_status.is_(None), + ~model.migration_status.startswith('target:')), + ~sql.exists().where(and_(model.id == admin_model.volume_id, + ~admin_model.deleted, + admin_model.key == 'temporary', + admin_model.value == 'True') + ) + ) + ) if host: query = query.filter(_filter_host(model.host, host)) diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 77cd15581ac..7223bafe4e7 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -554,6 +554,28 @@ class DBAPIVolumeTestCase(BaseTest): self.ctxt, 'project', skip_internal=skip_internal) self.assertEqual((count, gigabytes), result) + @ddt.data((True, THREE_HUNDREDS, THREE), + (False, THREE_HUNDREDS + ONE_HUNDREDS, THREE + 1)) + @ddt.unpack + def test__volume_data_get_for_project_temporary(self, skip_internal, + gigabytes, count): + for i in range(3): + db.volume_create(self.ctxt, + {'project_id': 'project', + 'size': ONE_HUNDREDS, + 'host': 'h-%d' % i, + 'volume_type_id': fake.VOLUME_TYPE_ID}) + # This is a temporary volume + db.volume_create(self.ctxt, {'project_id': 'project', + 'size': ONE_HUNDREDS, + 'host': 'h-%d' % i, + 'volume_type_id': fake.VOLUME_TYPE_ID, + 'admin_metadata': {'temporary': 'True'}}) + + result = sqlalchemy_api._volume_data_get_for_project( + self.ctxt, 'project', skip_internal=skip_internal) + self.assertEqual((count, gigabytes), result) + def test_volume_data_get_for_project_with_host(self): db.volume_create(self.ctxt, {'project_id': fake.PROJECT_ID, diff --git a/releasenotes/notes/quota-sync-temporary-b4103ebc2c484c89.yaml b/releasenotes/notes/quota-sync-temporary-b4103ebc2c484c89.yaml new file mode 100644 index 00000000000..13193d88419 --- /dev/null +++ b/releasenotes/notes/quota-sync-temporary-b4103ebc2c484c89.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Bug #1919161 `_: Fix + automatic quota refresh to correctly account for temporary volumes. During + some cinder operations, such as create a backup from a snapshot, temporary + volumes are created and are not counted towards quota usage, but the sync + mechanism was counting them, thus incorrectly updating volume usage.