ML2's assumptions about transactions should be explicit

[Kevin Benton]

The create/update/delete of the core resource in ML2 all assume that they will not be called inside of a transaction because they notify drivers with a postcommit call before returning. Calling ML2 with a session already in a transaction leads to unexpected behavior (e.g. no notifications to driver on a rollback) so we should enforce this assumption in code.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/275110
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

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

commit afe1a834000d33900b8646d308fa26fa807a2ca0
Author: Kevin Benton <email address hidden>
Date: Tue Mar 1 10:47:00 2016 -0800

    Block delete_(network|subnet) transactioned calls

    These functions assume that they won't be called inside of a
    transaction for two reasons. The first is an infinite loop that
    will only terminate if it can actually retrieve the latest
    information from the database. The second is that ML2's post-commit
    operations are expected to occur after data was actually committed
    so mechanism drivers that do DB lookups using a different session
    may break.

    This adds an explicit guard that prevents these two functions from
    being called within an active DB session. The rest of the ML2
    functions should eventually be converted to this style as well
    (see the partial bug).

    Related-Bug: #1551958
    Partial-Bug: #1540844
    Change-Id: I5c00b186585369ef6c8e2b9cb5a43b8bba0e5a7c

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit a2ceec28647d835aaa5228578537230ff571cee3
Author: Kevin Benton <email address hidden>
Date: Wed Apr 13 05:20:00 2016 -0700

    Don't update DHCP ports in a transaction

    Updating ports is incompatible with the transaction
    semantics that happen inside of ML2. See partial-bug for
    more details.

    This transaction served no purpose wrapped around the port
    update because there should only be one port per agent per
    network and if that port fails to update, the code to delete
    the binding won't be hit anyway.

    Change-Id: Ica2c5a19e10ae1773e49537b5ff4ae0eacaeb388
    Partial-Bug: #1540844

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/314022

[OpenStack Infra (hudson-openstack)]

Change abandoned by John Schwarz (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/314022

[OpenStack Infra (hudson-openstack)]

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/305804
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/275110
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[Armando Migliaccio (armando-migliaccio)]

ping?

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit e770c868f87f7053b8e282bcdbd2ce1a00a34b7f
Author: Kevin Benton <email address hidden>
Date: Thu May 12 09:17:56 2016 -0700

    Ensure most of ML2's core methods not in transaction

    This adds a check to each of the core methods that other callers
    (e.g. service plugins) may use to manipulate core resources. This
    check prevents them from passing in a context that is already part
    of an ongoing DB session because we do not want DB rollbacks to be
    allowed after the ML2 plugin calls postcommit methods on drivers.

    All of the core methods are protected except for create_port and
    update_port. This was left out because of a few particularily deeply
    nested calls to the port methods from the L3 code that will be
    addressed in change I5aa099c2264636336ab0b76c0826b506e2dc44b6.

    For more details, read the devref added by this patch.

    Partial-Bug: #1540844
    Change-Id: I9924600c57648f7eccaa5abb6979419d9547a2ff

[OpenStack Infra (hudson-openstack)]

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

commit b25f6448d530b51eea4a36b3d087515e8495f53f
Author: Kevin Benton <email address hidden>
Date: Thu May 12 09:17:56 2016 -0700

    Ensure ML2's create/update_port methods not in transaction

    This adds a check to the ML2 create and update port methods which
    are called by other services to manipulate ports. This check prevents
    them from passing in a context that is already part of an ongoing DB
    session because we do not want DB rollbacks to be allowed after the
    ML2 plugin calls postcommit methods on drivers.

    This also adds a temporary hack to set an attribute on the context
    to skip this check to accomadate two L3 code-paths and a subnet
    code-path that have port manipulations entangled in transactions.
    This attribute will ultimately be removed once these paths are
    refactored.

    Closes-Bug: #1540844
    Change-Id: I5aa099c2264636336ab0b76c0826b506e2dc44b6

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 9.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/395972

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/396013

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/396015

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/395972
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=202564cd643bb6c18ea2cd7a3c66617365fa0c72
Submitter: Jenkins
Branch: master

commit 202564cd643bb6c18ea2cd7a3c66617365fa0c72
Author: Kevin Benton <email address hidden>
Date: Mon Nov 14 14:55:41 2016 -0800

    Fix ML2, base db plugin update_subnet for transactions

    This refactors the DB base plugin update_subnet method into
    precommit and postcommit methods so a core plugin can avoid
    executing the postcommit methods while it has an open
    transaction. Backwards compatibility is preserved for plugins
    which do not need this.

    This is important because the update_subnet method in the
    base plugin performs operations that shouldn't be inside
    of an uncommited transaction, such as updating ports,
    generating AMQP notifications, and generating AFTER_UPDATE
    callback events. Until this patch, ML2 was performing the
    entire base plugin update_subnet operation in a transaction.

    This is also necessary for the facade and push-notification
    switch because ongoing db sessions will not be used for
    core methods and notification generation.

    Related-Bug: #1540844
    Partially-Implements: blueprint push-notifications
    Partially-Implements: blueprint enginefacade-switch
    Change-Id: Ib825a9e0c0c6693a4e708ae6ce362775c70d843d

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/396015
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=03f7ec38b6ebcb4badbf04ed5314b8db9f2d941b
Submitter: Jenkins
Branch: master

commit 03f7ec38b6ebcb4badbf04ed5314b8db9f2d941b
Author: Kevin Benton <email address hidden>
Date: Thu Nov 10 00:30:42 2016 -0800

    Use callbacks to create DVR floating GW port

    This stops the DVR mixin from overriding the '_update_fip_assoc'
    function in order to create its agent floating IP gateway ports.
    It instead now subscribes to the floating IP AFTER_UPDATE event
    to create these ports.

    Additionally, it fixes the semantics of the floating IP AFTER_UPDATE
    event so it's emitted after the update is committed to the DB
    so it's safe to call core plugin mutation methods and external systems.

    In addition to fixing the callback semantics, this is part of the
    effort to eliminate GUARD_TRANSACTION in-tree, which
    should pave the way for ML2 to adopt the new engine facade and for
    push notifications to safely transmit the latest DB state.

    Related-Bug: #1540844
    Partially-Implements: blueprint push-notifications
    Partially-Implements: blueprint enginefacade-switch
    Change-Id: I93e4d847f96707b17c4a7dfdb3bbf81d062fe3fb

[OpenStack Infra (hudson-openstack)]

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

commit c9358687cae598143cad20fffd16c6c191985ca4
Author: Kevin Benton <email address hidden>
Date: Wed Nov 9 23:25:01 2016 -0800

    Separate floating IP port creation from transaction

    This moves the floating IP port creation outside of the transaction
    that creates the floating IP record. This eliminates the use of one
    of the GUARD_TRANSACTION flags to prepare us for the enginefacade
    switch and correct event notification semantics for push
    notifications.

    Note that this introduces a small window where the server can die
    mid-process and leave an orphaned port without a floating IP. This
    is similar to what can happen for router gateway ports. The intention
    is to address these in a follow-up patch with a periodic garbage
    collection task that will look for floating IP ports with a device
    ID of 'PENDING' since the tenant cannot delete them on their own.

    Related-Bug: #1540844
    Partially-Implements: blueprint push-notifications
    Partially-Implements: blueprint enginefacade-switch
    Change-Id: Ia4c34c6654a5bfb64fbf06b11b0a29b018c6854f

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/396014
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=5ea4c986d3532ac519c7a2758d8a7e162c5e5bb3
Submitter: Jenkins
Branch: master

commit 5ea4c986d3532ac519c7a2758d8a7e162c5e5bb3
Author: Kevin Benton <email address hidden>
Date: Wed Nov 9 23:45:16 2016 -0800

    Move DVR fip agent gw port create out of transaction

    This shouldn't be happening in a transaction. It creates
    port resources in ML2 that need to have correct commit
    semantics.

    This refactors it to leverage the callback framework and
    unifies the codepath for creating these DVR ports after
    a router creation and after a router update with a migration.

    Related-Bug: #1540844
    Partially-Implements: blueprint push-notifications
    Partially-Implements: blueprint enginefacade-switch
    Change-Id: I15d8d32b54087e4a2ca1bfa74b46fa95adc4b95d

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/397499
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=6948467b770df68cb96877f3e0e710650ad63fd4
Submitter: Jenkins
Branch: master

commit 6948467b770df68cb96877f3e0e710650ad63fd4
Author: Kevin Benton <email address hidden>
Date: Mon Nov 14 19:05:16 2016 -0800

    Add janitor to cleanup orphaned fip ports

    This adds a janitor worker to the L3 DB module that
    will run every 5 minutes looking for floating IP ports
    with the device_id of 'PENDING'. If it finds any, it
    will keep track of the port ID to see if any stay in
    'PENDING' with the next iteration.

    If the device ID is still PENDING after 5 minutes, it
    means one of two things has happened. Either the server
    died after creating the floating IP port, but before
    creating the floating IP itself; or, it died after creating
    the floating IP port and the floating IP record, but before
    updating the device_id of the floating IP port to the
    floating IP ID.

    The janitor handles both cases by deleting the floating IP
    port if it has no associated floating IP and by updating
    the floating IP port device ID if it does have an associated
    floating IP.

    Related-Bug: #1540844
    Closes-Bug: #1648098
    Change-Id: I684a822553a5a0c54513ca7d20ccaf3c74180593