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 6ea945e3b1)
(cherry picked from commit 416290f193)
This commit is contained in:
melanie witt 2019-10-11 21:08:33 +00:00
parent 844209b064
commit a80eb82f67
2 changed files with 12 additions and 19 deletions

View File

@ -129,13 +129,8 @@ $MANAGE db online_data_migrations
# creation of a new deleted marker instance.
set +e
archive_deleted_rows $conf
set -e
# Verify whether online data migrations run after archiving will succeed.
# See for more details: https://bugs.launchpad.net/nova/+bug/1824435
$MANAGE db online_data_migrations
rc=$?
set -e
if [[ $rc -ne 2 ]]; then
echo "Expected return code 2 from online_data_migrations until bug 1824435 is fixed"
exit 2
fi

View File

@ -1776,7 +1776,7 @@ def instance_create(context, values):
values - dict containing column values.
"""
security_group_ensure_default(context)
default_group = security_group_ensure_default(context)
values = values.copy()
values['metadata'] = _metadata_refs(
@ -1804,21 +1804,19 @@ def instance_create(context, values):
instance_ref['extra'].update(values.pop('extra', {}))
instance_ref.update(values)
def _get_sec_group_models(security_groups):
models = []
default_group = _security_group_ensure_default(context)
if 'default' in security_groups:
models.append(default_group)
# Generate a new list, so we don't modify the original
security_groups = [x for x in security_groups if x != 'default']
if security_groups:
models.extend(_security_group_get_by_names(
context, security_groups))
return models
# Gather the security groups for the instance
sg_models = []
if 'default' in security_groups:
sg_models.append(default_group)
# Generate a new list, so we don't modify the original
security_groups = [x for x in security_groups if x != 'default']
if security_groups:
sg_models.extend(_security_group_get_by_names(
context, security_groups))
if 'hostname' in values:
_validate_unique_server_name(context, values['hostname'])
instance_ref.security_groups = _get_sec_group_models(security_groups)
instance_ref.security_groups = sg_models
context.session.add(instance_ref)
# create the instance uuid to ec2_id mapping entry for instance