diff --git a/nova/db/main/migrations/versions/d60bddf7a903_add_constraint_instance_share_avoid_.py b/nova/db/main/migrations/versions/d60bddf7a903_add_constraint_instance_share_avoid_.py new file mode 100644 index 0000000000..4f2a157390 --- /dev/null +++ b/nova/db/main/migrations/versions/d60bddf7a903_add_constraint_instance_share_avoid_.py @@ -0,0 +1,75 @@ +# 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. + +"""add_constraint_instance_share_avoid_duplicates + +Revision ID: d60bddf7a903 +Revises: 13863f4e1612 +Create Date: 2024-03-06 17:05:29.361678 +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'd60bddf7a903' +down_revision = '13863f4e1612' +branch_labels = None +depends_on = None + + +def upgrade(): + op.drop_table("share_mapping") + op.create_table( + "share_mapping", + sa.Column("created_at", sa.DateTime), + sa.Column("updated_at", sa.DateTime), + sa.Column( + "id", + sa.BigInteger().with_variant(sa.Integer, "sqlite"), + primary_key=True, + autoincrement=True, + nullable=False, + ), + sa.Column("uuid", sa.String(36)), + sa.Column( + "instance_uuid", + sa.String(length=36), + sa.ForeignKey( + "instances.uuid", name="share_mapping_instance_uuid_fkey" + ), + ), + sa.Column("share_id", sa.String(length=36)), + sa.Column("status", sa.String(length=32)), + sa.Column("tag", sa.String(48)), + sa.Column("export_location", sa.Text), + sa.Column("share_proto", sa.String(32)), + sa.UniqueConstraint( + "instance_uuid", + "share_id", + name="uniq_key_pairs0instance_uuid0share_id", + ), + sa.UniqueConstraint( + "instance_uuid", + "tag", + name="uniq_key_pairs0instance_uuid0tag", + ), + sa.Index("share_idx", "share_id"), + sa.Index( + "share_mapping_instance_uuid_share_id_idx", + "instance_uuid", + "share_id", + ), + mysql_engine="InnoDB", + mysql_charset="utf8", + ) diff --git a/nova/db/main/models.py b/nova/db/main/models.py index 3fbe209049..903ef267c9 100644 --- a/nova/db/main/models.py +++ b/nova/db/main/models.py @@ -761,6 +761,16 @@ class ShareMapping(BASE, NovaBase): sa.Index('share_idx', 'share_id'), sa.Index('share_mapping_instance_uuid_share_id_idx', 'instance_uuid', 'share_id'), + sa.UniqueConstraint( + "instance_uuid", + "share_id", + name="uniq_key_pairs0instance_uuid0share_id", + ), + sa.UniqueConstraint( + "instance_uuid", + "tag", + name="uniq_key_pairs0instance_uuid0tag", + ), ) # sqlite> create table my_table(id bigint primary key AUTOINCREMENT, # name text); diff --git a/nova/tests/unit/db/main/test_api.py b/nova/tests/unit/db/main/test_api.py index 87fcd00b3b..d8dcd5734c 100644 --- a/nova/tests/unit/db/main/test_api.py +++ b/nova/tests/unit/db/main/test_api.py @@ -4288,6 +4288,83 @@ class ShareMappingDBApiTestCase(test.TestCase): self._compare( share, expected_share_mappings_after_update[share.share_id]) + def test_share_mapping_duplicate(self): + ctxt = context.get_admin_context() + expected_share_mappings = self.create_test_data(ctxt) + + share_mappings = db.share_mapping_get_all(ctxt) + self.assertEqual(len(share_mappings), 2) + for share in share_mappings: + self._compare( + share, expected_share_mappings[share.share_id]) + + # Assuming we have a race condition, make sure we cannot create a + # duplicate entry with the same instance_uuid and share_id + with mock.patch( + "nova.db.main.api.share_mapping_get_by_instance_uuid_and_share_id" + ) as mock_db: + mock_db.return_value = None + + exc = self.assertRaises( + db_exc.DBDuplicateEntry, + db.share_mapping_update, + ctxt, + uuid='fake-uuid3', + instance_uuid='fake-instance-uuid1', + share_id='1', + status='attached', + tag='fake-tag3', + export_location='fake-export_location1', + share_proto='NFS' + ) + + self.assertIn( + ( + "UNIQUE constraint failed: " + "share_mapping.instance_uuid, share_mapping.share_id" + ), + str(exc), + ) + + def test_share_mapping_using_same_tag(self): + ctxt = context.get_admin_context() + expected_share_mappings = self.create_test_data(ctxt) + + share_mappings = db.share_mapping_get_all(ctxt) + self.assertEqual(len(share_mappings), 2) + for share in share_mappings: + self._compare( + share, expected_share_mappings[share.share_id]) + + # Avoid the same tag name with the same instance. Resulting with: + # libvirt.libvirtError: unsupported configuration: filesystem target + # 'my-share' specified twice + with mock.patch( + "nova.db.main.api.share_mapping_get_by_instance_uuid_and_share_id" + ) as mock_db: + mock_db.return_value = None + + exc = self.assertRaises( + db_exc.DBDuplicateEntry, + db.share_mapping_update, + ctxt, + uuid='fake-uuid3', + instance_uuid='fake-instance-uuid1', + share_id='3', + status='attached', + tag='fake-tag1', + export_location='fake-export_location1', + share_proto='NFS' + ) + + self.assertIn( + ( + "UNIQUE constraint failed: " + "share_mapping.instance_uuid, share_mapping.tag" + ), + str(exc), + ) + def test_share_mapping_get_by_share_id(self): ctxt = context.get_admin_context() expected_share_mappings = self.create_test_data(ctxt) diff --git a/nova/tests/unit/db/main/test_migrations.py b/nova/tests/unit/db/main/test_migrations.py index f3f46758d8..8619124d29 100644 --- a/nova/tests/unit/db/main/test_migrations.py +++ b/nova/tests/unit/db/main/test_migrations.py @@ -364,6 +364,16 @@ class NovaMigrationsWalk( self.assertForeignKeyExists( connection, 'share_mapping', 'instance_uuid') + def _pre_upgrade_d60bddf7a903(self, connection): + # Verifying the presence or absence of the uniqueness constraint + # does not seem trivial, especially for SQLite. Instead of checking + # here, the test "test_share_mapping_duplicate" ensures that we cannot + # introduce two identical keys into the share_mapping table. + pass + + def _check_d60bddf7a903(self, connection): + pass + def test_single_base_revision(self): """Ensure we only have a single base revision.