Alembic script must use named constraints to support future upgrades

Bug #1665066 reported by Jim Baker
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
craton
Fix Released
Critical
Jim Baker

Bug Description

Currently the Alembic migration script does not name all foreign key constraints. Example of current usage that needs to be fixed:

        sa.ForeignKeyConstraint(
            ['variable_association_id'], ['variable_association.id'],
            'fk_regions_variable_association')

Alternative usage, which also is much closer to how such FK constraints are specified in the models file:

    op.create_table(
        'hosts',
        sa.Column(
            'id', sa.Integer,
            sa.ForeignKey(
                'devices.id', name='fk_hosts_devices', ondelete='cascade'),
            primary_key=True)
    )

Without such naming, we have two problems:

1. Upgrades are harder if we cannot drop constraints by name, then re-instate
2. Potential conflicts with other FKs, as will be seen when we support RBAC scoped role assignments.

To ensure future migration support, this needs to be fixed in the CMDB scope, but it's an easy fix to perform (if a bit tedious).

Jim Baker (jimbaker)
Changed in craton:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to craton (master)

Fix proposed to branch: master
Review: https://review.openstack.org/441644

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to craton (master)

Reviewed: https://review.openstack.org/441644
Committed: https://git.openstack.org/cgit/openstack/craton/commit/?id=555d0c885692200b6ab3dbb3adf780784e389183
Submitter: Jenkins
Branch: master

commit 555d0c885692200b6ab3dbb3adf780784e389183
Author: Jim Baker <email address hidden>
Date: Wed Mar 29 11:25:22 2017 -0600

    Updates Alembic migration to better match SQLAlchemy models

    The original Alembic migration script was autogenerated, then
    subsequently modified by hand. Between these two aspects, some
    correspondence to the corresponding SQLAlchemy models was lost:

    * Columns were specified independently of corresponding foreign key
      and primary key constraints; besides making it more difficult to
      follow the code between models and the migration script, there was a
      loss of integrity constraints: a variable association must exist for
      every entity that mixes in VariableMixin as a base was not being
      enforced at the database level with an appropriate not-null; this
      causes problems for use of variables for any such object that is not
      constructed by models code, most notably via direct usage of the
      database. (See specifically
      https://bugs.launchpad.net/craton/+bug/1668251, fixed with this
      change.)

    * The column created_at would always be set by model-using code; but
      this was not enforced at the database level by ensuring it was not
      null.

    * Children should be deleted if the parent is deleted (we do not
      support "soft delete" in Craton).

    In addition, all constraints are now named (the original intent of
    this change, so as to ensure the feasibility of future migrations),
    and they are also named in a consistent fashion.

    Note that this change does not support many-to-many deletions (as seen
    in https://bugs.launchpad.net/craton/+bug/1668308), but SQLAlchemy
    does provide support for this at the model level. A future change can
    address this without requiring Alembic support.

    There's no direct testing of this change, but it is implicitly, and
    rather strongly, tested by the overall functional tests that are used;
    and to a lesser extent by some of the unit tests. This is in part
    because this change has strengthened constraints, not weakened them.

    Change-Id: I1f84c29610127de12c292a210fd003ae07bd6462
    Closes-bug: 1665066
    Closes-bug: 1668251

Changed in craton:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.