instance_group_delete() filters related entities by group UUID rather than by ID

[Roman Podoliaka]

Table 'instance_groups' has two columns to unambiguously identify the specific row:
1) id (Integer, primary_key=True, autoincrement=True)
2) uuid (String)

The former is used internally to bind related entities by FKs (InstanceGroupMember, InstanceGroupPolicy and InstanceGroupMetadata), and the latter is accepted by public DB API methods (this must be a miss in the DB schema design, because uuid could be easily used for both use cases).

Having two 'id' columns is both misleading and error-prone. E. g. instance_group_delete() deletes the instance group (and all related entities) given its UUID value:

def instance_group_delete(context, group_uuid):
    """Delete an group."""
    session = get_session()
    with session.begin():
        count = _instance_group_get_query(context,
                                          models.InstanceGroup,
                                          models.InstanceGroup.uuid,
                                          group_uuid,
                                          session=session).soft_delete()
        if count == 0:
            raise exception.InstanceGroupNotFound(group_uuid=group_uuid)

        # Delete policies, metadata and members
        instance_models = [models.InstanceGroupPolicy,
                           models.InstanceGroupMetadata,
                           models.InstanceGroupMember]
        for model in instance_models:
            model_query(context, model, session=session).\
                    filter_by(group_id=group_uuid).\
                    soft_delete()

Related entities are filtered by 'group_id' column, but 'group_uuid' value is passed. Despite that these two columns are of different types, this statement is executed successfully on MySQL and SQLite (though WHERE clause never evaluates to True, so related entities aren't actually deleted), but fails on PostgreSQL, which is more strict when checking data types of values.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/41408
Committed: http://github.com/openstack/nova/commit/767fb98b885327d1ad1cd380682ae4745aa78387
Submitter: Jenkins
Branch: master

commit 767fb98b885327d1ad1cd380682ae4745aa78387
Author: Roman Podolyaka <email address hidden>
Date: Mon Aug 12 14:46:53 2013 +0300

    Fix instance_group_delete() DB API method

    Table 'instance_groups' has two columns that can be used to
    unambigously identify the specific row:
      - 'id' (Integer, primary_key=True, autoincrement=True)
      - 'uuid' (String)

    The former is used internally to bind related instances by FKs
    (InstanceGroupMember, InstanceGroupPolicy and InstanceGroupMetadata),
    and the latter is accepted by public methods of DB API. This must
    be a miss in DB schema design, because 'uuid' could be used for both
    use cases.

    instance_group_delete() deletes the instance group (and all related
    instances) given its UUID value. When related entities are deleted
    they are filtered by the FK value - the 'group_id' column, however,
    the 'group_uuid' value is passed.

    Such DELETE statement is executed successfully (though WHERE clause never
    evaluates to True, so the query is logically incorrect) on MySQL and SQLite,
    which are often too 'soft' when checking data types of passed values.

    This patch fixes the instance_group_delete() method and renames the 'group_uuid'
    argument of _instance_group_model_get_query() to 'group_id', because this one
    is actually always called with id value rather than with uuid.

    Closes-Bug: #1211307

    Change-Id: Ic8bcb4b218f570f26420904aeb74eb14567235fb