de-duplicate route_targets and {import,export}_rts

If we do not do it now, then doing it later will be a real pain because the db content would have to be deduplicated for a proper migration.

However, giving more thinking to that, I'm not even sure we should do this de-duplication.
The reason is that with the deduplication, when an API user reads an object that had been created earlier, what he will get for the route_target fields will be different from what he will have specified initially.

I am not sure that the possible confusion coming from this is worth the very minor optimisation in the DB fields.

So the best plan seems to keep on storing every RT informations in the db as it has been provided during the creation of the bgpvpn.

The pain will to the driver. It will have to smartly process every RTs to generate the correct bgp request.

If we agree on that, then we only have to update the spec, am I right?

The pain on the driver side should be very minor.

We already have code in the bagpipe driver to consolidate export RTs from route_targets and export_targets, and import RTs from route_targets and import_targets. If useful, we can make this code available to all drivers.

And yes, if we agree to keep code as-is, the only thing that needs an update is the spec.

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

I have always been wondering why we make it so complicated. Why not just remove the route_targets attribute from the API and just specify import_rts and export_rts as independent lists. A backend can easily compute the intersection of both when it really needs the list of route targets that are both im and exported.

Jan: we think it is important to make it simple to address the simple and most common use case where export-RTs==import-RTs.

Reviewed: https://review.openstack.org/256300
Committed: https://git.openstack.org/cgit/openstack/networking-bgpvpn/commit/?id=8c740c06464fd9782311c20757424d3bfb42e78f
Submitter: Jenkins
Branch: master

commit 8c740c06464fd9782311c20757424d3bfb42e78f
Author: Mathieu Rohon <email address hidden>
Date: Fri Dec 11 09:20:37 2015 +0000

    Update the spec : remove RTs consolidation part

    RTs consolidation specified in the spec is currently not implemented.
    This consolidation will have to be done at the driver level,
    not at the plugin level.

    Change-Id: I3b3cbbf0846f8caadb4e7ce105bd33eac460dfe9
    Closes-bug: 1478998

I understand. Simple for the user, cumbersome for the backend developer.
Anyway, no strong objections. Go ahead.
/Jan

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Thomas Morin
> Sent: Friday, 11 December, 2015 14:43
> To: Jan Scheurich
> Subject: [Bug 1478998] Re: de-duplicate route_targets and {import,export}_rts
>
> Jan: we think it is important to make it simple to address the simple and most common use case where export-RTs==import-RTs.
>
> --
> You received this bug notification because you are subscribed to bgpvpn.
> Matching subscriptions: bgpvpn bugs
> https://bugs.launchpad.net/bugs/1478998
>
> Title:
> de-duplicate route_targets and {import,export}_rts
>
> Status in bgpvpn:
> In Progress
> Status in bgpvpn juno series:
> Confirmed
> Status in bgpvpn kilo series:
> Confirmed
> Status in bgpvpn liberty series:
> In Progress
>
> Bug description:
> API Spec text says:
>
> ------------------
> The implementation will rely on three tables:
>
> * one for BGPVPN objects
> * one to define the 1-n relation ship between a BGPVPN and a set of Networks
> * one to define the 1-n relation ship between a BGPVPN and a set of Routers
>
> These two last tables will not rely on foreign keys with the Neutron-owned
> tables for Networks and Routers.
>
> The information stored in these tables will reflect what is exposed on the
> API, with an exception for route_targets:
>
> * this list will not be present in the database
>
> * on an API request to create or modify a BGPVPN: the route_targets
> parameter will be merged without duplicates with import_rts before storing
> import_rts, ditto for export_rts
>
> * on an API request to show a BGPVPN:
>
> * route_targets will be synthesized to include RTs present in both
> export_rts and import_rts
>
> * import_targets will contain only RTs not in common with export_rts
> * export_targets will contain only RTs not in common with import_rts
> ----------------------
>
> The current code does not yet do the above.
> What is missing :
> - remove the route_targets field from the db
> - on API create and update, populate import_rts and export_rts taking into account route_targets
> - on API get, recreate route_targets
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/bgpvpn/+bug/1478998/+subscriptions