Merge "Amend DB model add a unique constraint."

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",
+    )

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);

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)

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.