Remove FK on service_id and make service_id nullable

Now that we are sure that there are no longer queries to the
compute/service relationship, we can safely remove the foreign key.

Change-Id: I779a4ce9c3e1ebcd6ea1b7124c6b06bd129e7015
Implements: blueprint detach-service-from-computenode

nova/db/sqlalchemy/migrate_repo/versions/278_remove_service_fk_in_compute_nodes.py

@@ -0,0 +1,105 @@
+# Copyright (c) 2015 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    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.
+
+
+from migrate import ForeignKeyConstraint, UniqueConstraint
+from sqlalchemy import MetaData, Table
+from sqlalchemy.engine import reflection
+
+
+def _correct_sqlite_unique_constraints(migrate_engine, table):
+    # NOTE(sbauza): SQLAlchemy<1.0 doesn't provide the unique keys in the
+    # constraints field of the Table object, so it would drop them if we change
+    # either the scheme or the constraints. Adding them back to the Table
+    # object before changing the model makes sure that they are not dropped.
+    if migrate_engine.name != 'sqlite':
+        # other engines don't have this problem
+        return
+    inspector = reflection.Inspector.from_engine(migrate_engine)
+    constraints = inspector.get_unique_constraints(table.name)
+    constraint_names = [c.name for c in table.constraints]
+    for constraint in constraints:
+        if constraint['name'] in constraint_names:
+            # the constraint is already in the table
+            continue
+        table.constraints.add(
+            UniqueConstraint(*constraint['column_names'],
+                             table=table, name=constraint['name']))
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    compute_nodes = Table('compute_nodes', meta, autoload=True)
+    shadow_compute_nodes = Table('shadow_compute_nodes', meta, autoload=True)
+    services = Table('services', meta, autoload=True)
+
+    _correct_sqlite_unique_constraints(migrate_engine, compute_nodes)
+
+    # Make the service_id column nullable
+    compute_nodes.c.service_id.alter(nullable=True)
+    shadow_compute_nodes.c.service_id.alter(nullable=True)
+
+    for fk in compute_nodes.foreign_keys:
+        if fk.column == services.c.id:
+            # Delete the FK
+            fkey = ForeignKeyConstraint(columns=[compute_nodes.c.service_id],
+                                        refcolumns=[services.c.id],
+                                        name=fk.name)
+            fkey.drop()
+            break
+    for index in compute_nodes.indexes:
+        if 'service_id' in index.columns:
+            # Delete the nested index which was created by the FK
+            index.drop()
+            break
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    compute_nodes = Table('compute_nodes', meta, autoload=True)
+    shadow_compute_nodes = Table('shadow_compute_nodes', meta, autoload=True)
+    services = Table('services', meta, autoload=True)
+
+    _correct_sqlite_unique_constraints(migrate_engine, compute_nodes)
+
+    # Make the service_id field not nullable
+    # NOTE(sbauza): Beyond the point of this commit, service_id will not be
+    # updated, but previous commits still do. We can tho safely go back to
+    # a state where all the compute nodes are providing this field.
+    compute_nodes.c.service_id.alter(nullable=False)
+    shadow_compute_nodes.c.service_id.alter(nullable=False)
+
+    # Adding only FK if not existing yet
+    fkeys = {fk.parent.name: fk.column
+             for fk in compute_nodes.foreign_keys}
+    if 'service_id' in fkeys and fkeys['service_id'] == services.c.id:
+        return
+
+    # NOTE(sbauza): See 216_havana.py for the whole logic
+    if migrate_engine.name == 'postgresql':
+        # PostgreSQL names things like it wants (correct and compatible!)
+        fkey = ForeignKeyConstraint(columns=[compute_nodes.c.service_id],
+                                    refcolumns=[services.c.id])
+        fkey.create()
+    else:
+        # For MySQL we name our fkeys explicitly so they match Havana
+        fkey = ForeignKeyConstraint(columns=[compute_nodes.c.service_id],
+                                    refcolumns=[services.c.id],
+                                    name='fk_compute_nodes_service_id')
+        fkey.create()

nova/db/sqlalchemy/models.py

@@ -112,13 +112,7 @@ class ComputeNode(BASE, NovaBase):
             name="uniq_compute_nodes0host0hypervisor_hostname"),
     )
     id = Column(Integer, primary_key=True)
-    service_id = Column(Integer, ForeignKey('services.id'), nullable=False)
-    service = orm.relationship(Service,
-                           backref=orm.backref('compute_node'),
-                           foreign_keys=service_id,
-                           primaryjoin='and_('
-                                'ComputeNode.service_id == Service.id,'
-                                'ComputeNode.deleted == 0)')
+    service_id = Column(Integer, nullable=True)
 
     # FIXME(sbauza: Host field is nullable because some old Juno compute nodes
     # can still report stats from an old ResourceTracker without setting this

nova/tests/unit/db/test_migrations.py

@@ -831,6 +831,28 @@ def _post_downgrade_277(self, engine):
         self.assertIndexNotExists(engine, 'fixed_ips',
                                   'fixed_ips_deleted_allocated_updated_at_idx')
 
+    def _check_278(self, engine, data):
+        compute_nodes = oslodbutils.get_table(engine, 'compute_nodes')
+        self.assertEqual(0, len([fk for fk in compute_nodes.foreign_keys
+                                 if fk.parent.name == 'service_id']))
+        self.assertTrue(compute_nodes.c.service_id.nullable)
+
+    def _post_downgrade_278(self, engine):
+        compute_nodes = oslodbutils.get_table(engine, 'compute_nodes')
+        service_id_fks = [fk for fk in compute_nodes.foreign_keys
+                          if fk.parent.name == 'service_id'
+                          and fk.column.name == 'id']
+        self.assertEqual(1, len(service_id_fks))
+        self.assertFalse(compute_nodes.c.service_id.nullable)
+        if engine.name == 'postgresql':
+            # Only make sure that posgresql at least adds a name for the FK
+            self.assertIsNotNone(service_id_fks[0].name)
+        elif engine.name != 'sqlite':
+            # Erm, SQLA<1.0 doesn't return FK names for sqlite so we need to
+            # check only for other engines
+            self.assertEqual('fk_compute_nodes_service_id',
+                             service_id_fks[0].name)
+
 
 class TestNovaMigrationsSQLite(NovaMigrationsCheckers,
                                test_base.DbTestCase,