openstack/cinder
Code Issues Proposed changes

DB: Set quota resource property length to 300

Browse Source 
On change I6c30a6be750f6b9ecff7399dbb0aea66cdc097da we increased the
`resource` column of the quota_usages table from 255 to 300, because its
value is constructed from (prefix + volume_type_name), but the length of
`volume_type_name` can be up to 255 characters, so if we add a prefix
such as 'volumes_' or 'gigabytes_' to it we'll exceed the db length
limit of the `resource` column.

There are other 3 quota related tables (quotas, quota_classes,
reservations) that have a `resource` column, and they are all
referencing the same kind of thing, but they still have a maximum size
of 255 characters, so there will be things that we won't be able to do
when using a volume type with a 255 characters name. Some of the
operations we won't be able to do are setting a default quota limit for
it or migrate volumes using that volume type.

Related-Bug: #1798327
Related-Bug: #1608849
Closes-Bug: #1948962
Change-Id: I40546b20322443dc34556de4aababf33a230db78
This commit is contained in:
Gorka Eguileor 2021-10-27 18:46:53 +02:00
parent 1c9aac8f5d
commit 3a968212d6
5 changed files with 99 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/api/validation/parameter_types.py
View File

@ -269,7 +269,7 @@ quota_class_set = {
'type': 'object',
'format': 'quota_class_set',
'patternProperties': {
'^[a-zA-Z0-9-_:. ]{1,255}$': {
'^[a-zA-Z0-9-_:. ]{1,300}$': {
'type': ['integer', 'string'],
'pattern': '^[0-9]*$', 'minimum': -1, 'minLength': 1,
'maximum': constants.DB_MAX_INT

67
cinder/db/migrations/versions/b8660621f1b9_update_reservations_resource.py Normal file
View File

@ -0,0 +1,67 @@
# 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.
"""Update reservations resource
Revision ID: b8660621f1b9
Revises: 89aa6f9639f9
Create Date: 2021-10-27 17:25:16.790525
"""
from alembic import op
from oslo_log import log as logging
import sqlalchemy as sa
LOG = logging.getLogger(__name__)
# revision identifiers, used by Alembic.
revision = 'b8660621f1b9'
down_revision = '89aa6f9639f9'
branch_labels = None
depends_on = None
def upgrade():
connection = op.get_bind()
for table_name in ('quotas', 'quota_classes', 'reservations'):
table = sa.Table(table_name, sa.MetaData(), autoload_with=connection)
col = table.c.resource
# SQLite doesn't support altering tables, so we use a workaround
if connection.engine.name == 'sqlite':
with op.batch_alter_table(table_name) as batch_op:
batch_op.alter_column('resource',
existing_type=col.type,
type_=sa.String(length=300))
else:
# MySQL ALTER needs to have existing_type, existing_server_default,
# and existing_nullable or it will do who-knows-what
try:
op.alter_column(table_name, 'resource',
existing_type=col.type,
existing_nullable=col.nullable,
existing_server_default=col.server_default,
type_=sa.String(length=300))
except Exception:
# On MariaDB, max length varies depending on the version and
# the InnoDB page size [1], so it is possible to have error
# 1071 ('Specified key was too long; max key length is 767
# bytes"). Since this migration is to resolve a corner case,
# deployments with those DB versions won't be covered.
# [1]: https://mariadb.com/kb/en/library/innodb-limitations/#page-sizes # noqa
if not connection.engine.name == 'mysql':
raise
LOG.warning('Error in migration %s, Cinder still affected by '
'bug #1948962', revision)

6
cinder/db/sqlalchemy/models.py
View File

@ -802,7 +802,7 @@ class Quota(BASE, CinderBase):
# TODO(stephenfin): Add index=True
project_id = sa.Column(sa.String(255))
resource = sa.Column(sa.String(255), nullable=False)
resource = sa.Column(sa.String(300), nullable=False)
hard_limit = sa.Column(sa.Integer, nullable=True)
# TODO: (X release): Remove allocated, belonged to nested quotas
allocated = sa.Column(sa.Integer, default=0)
@ -822,7 +822,7 @@ class QuotaClass(BASE, CinderBase):
class_name = sa.Column(sa.String(255), index=True)
resource = sa.Column(sa.String(255))
resource = sa.Column(sa.String(300))
hard_limit = sa.Column(sa.Integer, nullable=True)
@ -886,7 +886,7 @@ class Reservation(BASE, CinderBase):
)
project_id = sa.Column(sa.String(255), index=True)
resource = sa.Column(sa.String(255))
resource = sa.Column(sa.String(300))
delta = sa.Column(sa.Integer, nullable=False)
# TODO(stephenfin): Add nullable=False

22
cinder/tests/unit/db/test_migrations.py
View File

@ -183,6 +183,10 @@ class MigrationsWalk(
'c92a3e68beed',
# Migration 89aa6f9639f9 doesn't fail because it's for a SQLAlquemy
# internal table, and we only check Cinder's tables.
# Increasing resource column max length to 300 is acceptable, since
# it's a backward compatible change.
'b8660621f1b9',
]
FORBIDDEN_METHODS = ('alembic.operations.Operations.alter_column',
'alembic.operations.Operations.drop_column',
@ -190,6 +194,8 @@ class MigrationsWalk(
'alembic.operations.BatchOperations.alter_column',
'alembic.operations.BatchOperations.drop_column')
VARCHAR_TYPE = sqlalchemy.types.VARCHAR
def setUp(self):
super().setUp()
self.engine = enginefacade.writer.get_engine()
@ -284,6 +290,22 @@ class MigrationsWalk(
# for its removal without creating it first, which is dumb
pass
def _pre_upgrade_b8660621f1b9(self, connection):
"""Test resource columns were limited to 255 chars before."""
for table_name in ('quotas', 'quota_classes', 'reservations'):
table = db_utils.get_table(connection, table_name)
self.assertIn('resource', table.c)
self.assertIsInstance(table.c.resource.type, self.VARCHAR_TYPE)
self.assertEqual(255, table.c.resource.type.length)
def _check_b8660621f1b9(self, connection):
"""Test resource columns can be up to 300 chars."""
for table_name in ('quotas', 'quota_classes', 'reservations'):
table = db_utils.get_table(connection, table_name)
self.assertIn('resource', table.c)
self.assertIsInstance(table.c.resource.type, self.VARCHAR_TYPE)
self.assertEqual(300, table.c.resource.type.length)
class TestMigrationsWalkSQLite(
MigrationsWalk,

6
releasenotes/notes/fix-resource-size-76e8ff25f07925f2.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
`Bug #1948962 <https://bugs.launchpad.net/cinder/+bug/1948962>`_: Fixed
operations that failed on volume types with 255 characters names (e.g. set
quota limits or volume migrate).