Handle SAWarning around allocations FK Constratins

We have started to notice an SAWarning from sqlalchemy indicating:

  SAWarning: Cannot correctly sort tables; there are unresolvable
      cycles between tables "allocations, nodes", which is usually
      caused by mutually dependent foreign key constraints.
      Foreign key constraints involving these tables will not be
      considered; this warning may raise an error in a future release.

Hunting this down, it appears to be the two data consistency Foreign
Key constraints in the "allocations" table where an allocation would
try to have a conductor_affinity value mapped to conductors.id
and also have a direct association to a node, which *also* had the
same constraint.

And then similarlly, mapping in reverse, asserting a fk constraint,
when nodes also had it's own constraint back on allocations.

Sort of a circular loop.

Anyhow, removes it, and adds a db migration to remove the two
constraints.

Change-Id: I5596008e4971a29c635c45b24cb85db2d0d13ed3

ironic/db/sqlalchemy/alembic/versions/d163df1bab88_remove_extra_fk_constraint_in_.py

@@ -0,0 +1,50 @@
+#    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.
+
+
+"""remove_extra_fk_constraint in allocations
+
+Revision ID: d163df1bab88
+Revises: 163040c5513f
+Create Date: 2023-06-23 11:20:10.904262
+
+"""
+
+
+from alembic import op
+from oslo_db import exception as db_exc
+from oslo_db.sqlalchemy import enginefacade
+
+
+# revision identifiers, used by Alembic.
+revision = 'd163df1bab88'
+down_revision = '163040c5513f'
+
+
+def upgrade():
+    engine = enginefacade.reader.get_engine()
+
+    if engine.dialect.name == 'mysql':
+        # Remove the outer level conductor_affinity constraint which pointed
+        # to the conductor.id field, when the allocation's table also already
+        # points to unique node constraints where a node also has a conductor
+        # affinity field on the conductor.id. Removing because this is
+        # expected to be come a hard error for SQLAlchemy at some point.
+        try:
+            op.drop_constraint('allocations_ibfk_1', 'allocations',
+                               type_="foreignkey")
+            op.drop_constraint('allocations_ibfk_2', 'allocations',
+                               type_="foreignkey")
+        except db_exc.DBNonExistentConstraint:
+            # This is to ignore this issue on newer deployments where
+            # key mappings may not already exist.
+            pass

ironic/db/sqlalchemy/alembic/versions/dd67b91a1981_add_allocations_table.py

@@ -44,8 +44,18 @@ def upgrade():
         sa.Column('candidate_nodes', types.JsonEncodedList(), nullable=True),
         sa.Column('extra', types.JsonEncodedDict(), nullable=True),
         sa.Column('conductor_affinity', sa.Integer(), nullable=True),
-        sa.ForeignKeyConstraint(['conductor_affinity'], ['conductors.id'], ),
-        sa.ForeignKeyConstraint(['node_id'], ['nodes.id'], ),
+        # NOTE(TheJulia): Commenting these out to remove the constraints
+        # as below we link nodes to allocations on a unique foreign key
+        # constraint mapping, and nodes also have the same conductor affinity
+        # constraint, which raises an SAWarning error as sqlalchemy cannot
+        # sort the data model relationships, and expects this to become an
+        # error at some point in the future. As such, since a node is
+        # generally a primary object, and allocations are more secondary
+        # relationship/association mapping objects, the two commented out
+        # lines are redundant. We'll remove the relationships in a migration
+        # as well and ignore errors if they are encountered for new installs.
+        # sa.ForeignKeyConstraint(['conductor_affinity'], ['conductors.id'], ),
+        # sa.ForeignKeyConstraint(['node_id'], ['nodes.id'], ),
         sa.PrimaryKeyConstraint('id'),
         sa.UniqueConstraint('name', name='uniq_allocations0name'),
         sa.UniqueConstraint('uuid', name='uniq_allocations0uuid'),

ironic/db/sqlalchemy/models.py

@@ -412,7 +412,7 @@ class Allocation(Base):
     id = Column(Integer, primary_key=True)
     uuid = Column(String(36), nullable=False)
     name = Column(String(255), nullable=True)
-    node_id = Column(Integer, ForeignKey('nodes.id'), nullable=True)
+    node_id = Column(Integer, nullable=True)
     state = Column(String(15), nullable=False)
     owner = Column(String(255), nullable=True)
     last_error = Column(Text, nullable=True)
@@ -421,8 +421,7 @@ class Allocation(Base):
     candidate_nodes = Column(db_types.JsonEncodedList)
     extra = Column(db_types.JsonEncodedDict)
     # The last conductor to handle this allocation (internal field).
-    conductor_affinity = Column(Integer, ForeignKey('conductors.id'),
-                                nullable=True)
+    conductor_affinity = Column(Integer, nullable=True)
 
 
 class DeployTemplate(Base):

releasenotes/notes/remove-excess-allocation-fk-constraints-0f59170f4d164a6e.yaml

@@ -0,0 +1,6 @@
+---
+upgrade:
+  - |
+    This release removes two internal foreign key constraints which were
+    redundant and which SQLAlchemy indicated may result in an error at some
+    point in time. No action is required by an operator for this.