diff --git a/cinder/api/validation/parameter_types.py b/cinder/api/validation/parameter_types.py index cb8203d548f..a01993f510e 100644 --- a/cinder/api/validation/parameter_types.py +++ b/cinder/api/validation/parameter_types.py @@ -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 diff --git a/cinder/db/migrations/versions/b8660621f1b9_update_reservations_resource.py b/cinder/db/migrations/versions/b8660621f1b9_update_reservations_resource.py new file mode 100644 index 00000000000..2e31479325b --- /dev/null +++ b/cinder/db/migrations/versions/b8660621f1b9_update_reservations_resource.py @@ -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) diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index 761d61ec965..0da4e97c357 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -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 diff --git a/cinder/tests/unit/db/test_migrations.py b/cinder/tests/unit/db/test_migrations.py index 567d1ba91bf..39b776063a3 100644 --- a/cinder/tests/unit/db/test_migrations.py +++ b/cinder/tests/unit/db/test_migrations.py @@ -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, diff --git a/releasenotes/notes/fix-resource-size-76e8ff25f07925f2.yaml b/releasenotes/notes/fix-resource-size-76e8ff25f07925f2.yaml new file mode 100644 index 00000000000..b8d10c7b99d --- /dev/null +++ b/releasenotes/notes/fix-resource-size-76e8ff25f07925f2.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1948962 `_: Fixed + operations that failed on volume types with 255 characters names (e.g. set + quota limits or volume migrate).