Bgpvpn plugin not having precommit callbacks to service-drivers for delete_xxx lifecycle

[Vivekanandan Narasimhan]

Bgpvpn plugin is not having precommit callbacks to service-drivers for delete_xxx lifecycle

The Bgpvpn plugin needs to be fixed to make precommit callbacks to service drivers for the following resource lifecycle:
a. delete_bgpvpn
b. delete_net_assoc
c. delete_router_assoc

[Thomas Morin (tmmorin-orange)]

Hi,

Can you:
- split the bug in two for the two distinct issues
- explain why these fixes are needed

Thanks!

[Thomas Morin (tmmorin-orange)]

Related change: https://review.openstack.org/#/c/333078/

[Vivekanandan Narasimhan (vivekanandan-narasimhan)]

Can you:
- split the bug in two for the two distinct issues

When the change is in the same BGPVPN Plugin and both the issues are related to only precommit problems, I tend to think it is better to just have this bug as a single placeholder.

- explain why these fixes are needed

The precommit should always be invoked for backend drivers in the same DB transaction as the parent plugin resource commit.

Further, OpenDaylight service driver for bgpvpn requires precommit invocations so that it can pre-create the journals to sync them with ODL Controller later on post-commmit.

[Thomas Morin (tmmorin-orange)]

It will be easier for us to track as separate issues:
- One item is, from the point of view of the hooks offered by the service to drivers, an additional feature required for ODL.
- The other item seems to be a bug, not related to these new hooks but common to all precommit hooks, and possibly impact other drivers. We might need to track this as a bug and backport a fix.

For the second issue, it will help us understand if you explain /why/ "precommit should always be invoked for backend drivers in the same DB transaction as the parent plugin resource commit" and why nested transactions are not good.

[Vivekanandan Narasimhan (vivekanandan-narasimhan)]

Hi Thomas,

Will use this bug for the first item.
Will create another bug for the second item.

I will push the second item as first review though since its a parent fix and
the first item can be addressed as a dependent review later.

Suggest if possible lets us have both the bugs backported to Ocata.

The precommit call to backend drivers must be in same transaction as the frontend DB, because
a. Backend drivers may need to append some additional information to the original resource (or commit extra information related to the original resource) but such information is of relevance only by the backend.
b. And the whole resource set (front + back) need to have guaranteed atomicity on the DB write.

[OpenStack Infra (hudson-openstack)]

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

[Thomas Morin (tmmorin-orange)]

Ok to split the second item in a separate bug.

Can you edit the bug summary of this bug to reflect that ?

[Vivekanandan Narasimhan (vivekanandan-narasimhan)]

The second item that states that the precommit invocations should be on the same DB transactions as the parent resource commit by the bgpvpn plugin is already in place.

In order to indicate the same, I removed the second item from the bug description.

[Vivekanandan Narasimhan (vivekanandan-narasimhan)]

The second item that I have removed from the bug description is provided here for historical reference:

"Also the existing precommit callbacks within the following must be changed not to use nested subtransactions:
a. create_bgpvpn
b. create_net_assoc
c. create_router_assoc
d. update_bgpvpn

The precommit invocations should be in the same DB Transaction as the parent DB transaction that commits to the relevant bgpvpn resource into the Openstack DB."

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/435651
Committed: https://git.openstack.org/cgit/openstack/networking-bgpvpn/commit/?id=6ee70a5f5e53c3fbaf00e7f1726a32625fe25dba
Submitter: Jenkins
Branch: master

commit 6ee70a5f5e53c3fbaf00e7f1726a32625fe25dba
Author: Vivekanandan Narasimhan <email address hidden>
Date: Sat Feb 18 09:32:08 2017 +0530

    Introduce precommit hooks for delete_bgpvpn_xxx

    Introduce precommit hooks for all delete_bgpvpn_xxx
    methods in the bgpvpn plugin.

    The precommit hooks introduced here will be used by
    ODL BGPVPN Version 2 driver to write to journal
    on precommits.

    Also subtransactions=True were missing for net_assoc and
    router_assoc methods inside bgpvpn_db and has been added
    as part of this fix.

    Change-Id: I422cfb5ff5c2a15c4a80430ba99e19b9ae768b06
    Closes-Bug: #1664637

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/449533

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/449533
Committed: https://git.openstack.org/cgit/openstack/networking-bgpvpn/commit/?id=09902d659c0f24c2800dca481e979dae51ee6c27
Submitter: Jenkins
Branch: stable/ocata

commit 09902d659c0f24c2800dca481e979dae51ee6c27
Author: Vivekanandan Narasimhan <email address hidden>
Date: Sat Feb 18 09:32:08 2017 +0530

    Introduce precommit hooks for delete_bgpvpn_xxx

    Introduce precommit hooks for all delete_bgpvpn_xxx
    methods in the bgpvpn plugin.

    The precommit hooks introduced here will be used by
    ODL BGPVPN Version 2 driver to write to journal
    on precommits.

    Also subtransactions=True were missing for net_assoc and
    router_assoc methods inside bgpvpn_db and has been added
    as part of this fix.

    Change-Id: I422cfb5ff5c2a15c4a80430ba99e19b9ae768b06
    Closes-Bug: #1664637
    (cherry picked from commit 6ee70a5f5e53c3fbaf00e7f1726a32625fe25dba)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/networking-bgpvpn 7.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/networking-bgpvpn 6.0.1 release.