Comment 25 for bug 1824435

Reviewed: https://review.opendev.org/692907
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=a80eb82f675ae4c933158fc097094cbbbb96f9dd
Submitter: Zuul
Branch: stable/stein

commit a80eb82f675ae4c933158fc097094cbbbb96f9dd
Author: melanie witt <email address hidden>
Date: Fri Oct 11 21:08:33 2019 +0000

    Remove redundant call to get/create default security group

    In the instance_create DB API method, it ensures the (legacy) default
    security group gets created for the specified project_id if it does
    not already exist. If the security group does not exist, it is created
    in a separate transaction.

    Later in the instance_create method, it reads the default security group
    back that it wrote earlier (via the same ensure default security group
    code). But since it was written in a separate transaction, the current
    transaction will not be able to see it and will get back 0 rows. So, it
    creates a duplicate default security group record if project_id=NULL
    (which it will be, if running nova-manage db online_data_migrations,
    which uses an anonymous RequestContext with project_id=NULL). This
    succeeds despite the unique constraint on project_id because in MySQL,
    unique constraints are only enforced on non-NULL values [1].

    To avoid creation of a duplicate default security group for
    project_id=NULL, we can use the default security group object that was
    returned from the first security_group_ensure_default call earlier in
    instance_create method and remove the second, redundant call.

    This also breaks out the security groups setup code from a nested
    method as it was causing confusion during code review and is not being
    used for any particular purpose. Inspection of the original commit
    where it was added in 2012 [2] did not contain any comments about the
    nested method and it appeared to either be a way to organize the code
    or a way to reuse the 'models' module name as a local variable name.

    Closes-Bug: #1824435

    [1] https://dev.mysql.com/doc/refman/8.0/en/create-index.html#create-index-unique
    [2] https://review.opendev.org/#/c/8973/2/nova/db/sqlalchemy/api.py@1339

     Conflicts:
     gate/post_test_hook.sh

    NOTE(melwitt): The conflict is because of the difference from Train in
    the patch below this one where we need to pass the cell1 config to the
    archive command because change
    If133b12bf02d708c099504a88b474dce0bdb0f00 is not in Stein.

    Change-Id: Idb205ab5b16bbf96965418cd544016fa9cc92de9
    (cherry picked from commit 6ea945e3b126879a86fe78d9c0537f5d258cf91a)
    (cherry picked from commit 416290f1938631a95aaaf92a41f455ebf75b0732)