Race condition exists for default security group creation

I have registered a BP: https://blueprints.launchpad.net/quantum/+spec/distributed-lock-resource-allocation for such kind of bugs.

Hi, I just submitted https://review.openstack.org/#/c/91805/. Mark, I don't know if this is the bug I asked you about a while back, may well be as I recall you saying something about overhauling some code to enforce this. I mean as was envisaged/described by the bluprint [1]. I then came across [2] which deals with the corresponding mac issue (though in code, no just on the model as slightly different situation) and there wasn't too much pushback there (though was ultimately abandoned...).

In any case, I made the review to also learn about the alembic migrations as I've never touched it before. Let me know if I should abandon this for whatever it is you have brewing to address it (unless it's a different bug...)

thanks, marios

[1] https://blueprints.launchpad.net/neutron/+spec/distributed-lock-resource-allocation
[2] https://review.openstack.org/#/c/34823/

This bug is specifically about creation of the security group called 'default'. Some questions:

* Do we really want to enforce that security group names are unique _per_tenant_? A lot of tests are failing with this migration because of duplicate security group names (this can be fixed obviously).
* If we do, we probably need to document it (will add !DocImpact) [1]
* How do we handle the case when someone tries to migrate who already has duplicate sg names for a given tenant_id? [2]

I just came across a relevant review @ [3]"Duplicate security group name cause fail to start instance" but this tried to enforce uniqueness in code, rather than the db schema.

[1] http://docs.openstack.org/api/openstack-network/2.0/content/security_groups.html
[2] https://review.openstack.org/#/c/91805/1/neutron/db/migration/alembic_migrations/versions/591bbc0502b2_enforce_unique_tenan.py
[3] https://review.openstack.org/#/c/79270/2

It is very rare at the gate, but looks like it can happen:
http://logstash.openstack.org/#eyJzZWFyY2giOiJtZXNzYWdlOiBcIlNlY3VyaXR5IGdyb3VwIGRlZmF1bHQgYWxyZWFkeSBleGlzdHMgZm9yIHByb2plY3RcIiIsImZpZWxkcyI6W10sIm9mZnNldCI6MCwidGltZWZyYW1lIjoiYWxsIiwiZ3JhcGhtb2RlIjoiY291bnQiLCJ0aW1lIjp7InVzZXJfaW50ZXJ2YWwiOjB9LCJzdGFtcCI6MTQwMzgwNzgyMDAxNX0=

It also can happen with the tempest vm boot stress (rare) :
https://github.com/openstack/tempest/blob/master/tempest/stress/actions/server_create_destroy.py

Hi, are we going to move on with the fix?
The patch is on gerrit, Marios, can you resurrect it?

Change abandoned by Salvatore Orlando (<email address hidden>) on branch: master
Review: https://review.openstack.org/91805
Reason: This patch has been dead enough that I think it's safe to abandon.
The author can resurrect it if needed.

Suggested solution https://review.openstack.org/135006

Change abandoned by Ann Kamyshnikova (<email address hidden>) on branch: master
Review: https://review.openstack.org/135006
Reason: I continue working on existing change as I see that people share my opinion here in thread. Seems that I'm wrong and disadvantages listed by others are more important. I will work on alternative.

Reviewed: https://review.openstack.org/142101
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=79c97120de9cff4d0992b5d41ff4bbf05e890f89
Submitter: Jenkins
Branch: master

commit 79c97120de9cff4d0992b5d41ff4bbf05e890f89
Author: Ann Kamyshnikova <email address hidden>
Date: Fri Dec 12 15:30:06 2014 +0300

    Default security group table

    This change prevents the race condition by enforcing a single default
    security group via new table default_security_group. It has tenant_id
    as primary key and security_group_id, which is id of default
    security group. Migration that inroduces this table has sanity check that
    verifies that there is no duplicate default security group in any
    tenant.

    This idea has come up from discussion in comments to
    https://review.openstack.org/135006

    DocImpact

    Closes-bug: #1194579

    Change-Id: Ifa8fbddd22bce4c50836cf443ebe10dff37443ef

Related fix proposed to branch: stable/juno
Review: https://review.openstack.org/156596

Change abandoned by Ihar Hrachyshka (<email address hidden>) on branch: stable/juno
Review: https://review.openstack.org/156596
Reason: That's an unreasonable and incomplete approach that we cannot take for stable releases. There seems to be no solution for Juno without db migration rules that are not available for backports in upstream, so abandoning the patch.