From 24dd74748d22232bd91147367c9ac6a6ccf25cc5 Mon Sep 17 00:00:00 2001 From: zhangbailin Date: Thu, 18 Oct 2018 03:44:12 -0400 Subject: [PATCH] Increase the length of resource property in quota_usages When updating the volume type, the length of the name is checked to be 0-255. If the user input length is 255 characters (e.g. volume_type_name='X*255'), when synchronizing the quota resource, the resource name needs to be updated to the form of 'resource + volume_type_name' in quota_usages [1], because the resource attribute in the DB is set to 'String(255)', the length limit is exceeded. Therefore, the resource column attribute in the quota_usages database table needs to be changed from 'String(255)' to 'String(300)'. [1]https://github.com/openstack/cinder/blob/master/cinder/db/sqlalchemy/api.py#L352 Closes-Bug: #1798327 Closes-Bug: #1608849 Change-Id: I6c30a6be750f6b9ecff7399dbb0aea66cdc097da --- ...ect_resource_attribute_for_quota_usages.py | 27 +++++++++++++++++++ cinder/db/sqlalchemy/models.py | 2 +- cinder/tests/unit/db/test_migrations.py | 13 +++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 cinder/db/sqlalchemy/migrate_repo/versions/127_change_project_resource_attribute_for_quota_usages.py diff --git a/cinder/db/sqlalchemy/migrate_repo/versions/127_change_project_resource_attribute_for_quota_usages.py b/cinder/db/sqlalchemy/migrate_repo/versions/127_change_project_resource_attribute_for_quota_usages.py new file mode 100644 index 00000000000..3e7a9cb2308 --- /dev/null +++ b/cinder/db/sqlalchemy/migrate_repo/versions/127_change_project_resource_attribute_for_quota_usages.py @@ -0,0 +1,27 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import MetaData, String, Table + + +def upgrade(migrate_engine): + """Increase the resource column size to the quota_usages table. + + The resource value is constructed from (prefix + volume_type_name), + but the length of volume_type_name is limited to 255, if we add a + prefix such as 'volumes_' or 'gigabytes_' to volume_type_name it + will exceed the db length limit. + """ + meta = MetaData(bind=migrate_engine) + + quota_usages = Table('quota_usages', meta, autoload=True) + quota_usages.c.resource.alter(type=String(300)) diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index 40c8c37a4bc..86e5afbe7cb 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -631,7 +631,7 @@ class QuotaUsage(BASE, CinderBase): id = Column(Integer, primary_key=True) project_id = Column(String(255), index=True) - resource = Column(String(255), index=True) + resource = Column(String(300), index=True) in_use = Column(Integer) reserved = Column(Integer) diff --git a/cinder/tests/unit/db/test_migrations.py b/cinder/tests/unit/db/test_migrations.py index c72262d0a21..e3443c79a15 100644 --- a/cinder/tests/unit/db/test_migrations.py +++ b/cinder/tests/unit/db/test_migrations.py @@ -115,6 +115,12 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): # NOTE : 104 modifies size of messages.project_id to 255. # This should be safe for the same reason as migration 87. 104, + # NOTE(brinzhang): 127 changes size of quota_usage.resource + # to 300. This should be safe for the 'quota_usage' db table, + # because of the 255 is the length limit of volume_type_name, + # it should be add the additional prefix before volume_type_name, + # which we of course allow *this* size to 300. + 127, ] if version not in exceptions: @@ -409,6 +415,13 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): volume_transfer = db_utils.get_table(engine, 'transfers') self.assertIn('no_snapshots', volume_transfer.c) + def _check_127(self, engine, data): + quota_usage_resource = db_utils.get_table(engine, 'quota_usages') + self.assertIn('resource', quota_usage_resource.c) + self.assertIsInstance(quota_usage_resource.c.resource.type, + self.VARCHAR_TYPE) + self.assertEqual(300, quota_usage_resource.c.resource.type.length) + def test_walk_versions(self): self.walk_versions(False, False) self.assert_each_foreign_key_is_part_of_an_index()