<div dir="ltr">See @PCM inline...<div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 26, 2015 at 4:44 AM Germy Lure <<a href="mailto:germy.lure@gmail.com">germy.lure@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>Maybe I missed some key points. But why we introduced vpn-endpoint groups here?</div></div></blockquote><div><br></div><div>@PCM For the multiple local subnet capabilities for IPSec, the existing API would need to be changed, so that we can specify 1+ local subnets, as part of the VPN connection. This would have been the simplest approach for updating IPSec connections, however, it makes it a point solution.</div><div><br></div><div>Other VPN flavors would also have similar "endpoint" specifications in their "connection" APIs.</div><div><br></div><div>The approach that I'm advocating, is to extract out some of the commonality between VPN flavors such that we can have some reuse.</div><div><br></div><div>Essentially, the idea is to break VPN into two parts. One is "what is connected" and the other is "how the connection is made".</div><div><br></div><div>For the former, the idea of an endpoint group is introduced. It provides a collection of endpoints that can be identified by a type (e.g. subnet, CIDR, network, vlan, ...) and an ID.</div><div><br></div><div>The latter would be VPN flavor specific, having all of the details needed for that type of VPN connection and would reference the needed endpoint group(s) by ID.</div><div><br></div><div>This separates the "what" from the "how".</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div><div>"ipsec-site-connection" for IPSec VPN only, "gre-connection" for GRE VPN only, and "mpls-connection" for MPLS VPN only. You see, different connections for different vpn types. Indeed, We can't reuse connection API.</div></div></div></blockquote><div><br></div><div>@PCM Correct. The "how" is VPN type specific. But we can have a common API for the "what".</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><br></div><div>Piece of the ref document(<a href="https://review.openstack.org/#/c/191944/" style="font-size:13px;line-height:19px" target="_blank">https://review.openstack.org/#/c/191944/</a>) like this:</div><div>"allowing subnets (local) and CIDRs (peer) to be used for IPSec, but routers, networks, and VLANs to be used for other VPN types (BGP, L2, direct connection)"</div><div><br></div><div>You see, different epg types for different vpn types. We can't reuse epg.</div></div></div></blockquote><div><br></div><div>@PCM We're not reusing the endpoint group itself for different types of VPN, we're reusing the API for different types of VPN. A common API that holds a collection of endpoints of a specific type.</div><div><br></div><div>You can look at the code out for review, for a feel for the implementation being worked on:  <a href="https://review.openstack.org/#/c/212692/3">https://review.openstack.org/#/c/212692/3</a></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><br></div><div>So, how we meet "The third goal, is to do this in a manner that the code can be reused for other flavors of VPN."?</div></div></div></blockquote><div><br></div><div>@PCM The code for the endpoint group API could be used for other VPN types. Instead of them creating table fields (and the corresponding db logic) for the endpoints of their connection, they can refer to the ID(s) from the endpoint groups table, and can add additional validation based on the VPN type.</div><div><br></div><div>FYI, I pushed up version 7 of the dev ref document yesterday.</div><div><br></div><div>Regards,</div><div><br></div><div>PCM</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 25, 2015 at 1:54 AM, Madhusudhan Kandadai <span dir="ltr"><<a href="mailto:madhusudhan.openstack@gmail.com" target="_blank">madhusudhan.openstack@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">My two cents..<br><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote"><span>On Mon, Aug 24, 2015 at 8:48 AM, Jay Pipes <span dir="ltr"><<a href="mailto:jaypipes@gmail.com" target="_blank">jaypipes@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Paul, comments inline...<span><br>
<br>
On 08/24/2015 07:02 AM, Paul Michali wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi,<br>
<br>
I'm working on the multiple local subnet feature for VPN (RFE<br>
<a href="https://bugs.launchpad.net/neutron/+bug/1459423" rel="noreferrer" target="_blank">https://bugs.launchpad.net/neutron/+bug/1459423</a>), with a developer<br>
reference document detailing the proposed process<br>
(<a href="https://review.openstack.org/#/c/191944/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/191944/</a>). The plan is to do this in<br>
two steps. The first is to add new APIs and database support for<br>
"endpoint groups" (see dev ref for details). The second is to modify the<br>
IPSec/VPN APIs to make use of the new information (and no longer use<br>
some older, but equivalent info that is being extended).<br>
<br>
I have a few process/procedural questions for the community...<br>
<br>
Q1) Should I do this all as one huge commit, as two commits (one for<br>
endpoint groups and one for modification to support multiple local<br>
subnets), or multiple (chained) commits (e.g. commit for each API for<br>
the endpoint groups and for each part of the multiple subnet change)?<br>
<br>
My thought (now) is to do this as two commits, with the endpoint groups<br>
as one, and multiple subnet groups as a second. I started with a commit<br>
for create API of endpoint (212692), and then did a chained commit for<br>
delete/show/list (215717), thinking they could be reviewed in pieces,<br>
but they are not that large and could be easily merged.<br>
</blockquote>
<br></span>
My advice would be 2 commits, as you have split them out.<span><br></span></blockquote><div><br></div></span><div>I would prefer to have two commits with end-point groups as one and modification to support multiple local subnets as another. This will be easy to troubleshoot when in need.<br></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Q2) If the two parts are done separately, should the "endpoint group"<br>
portion, which adds a table and API calls, be done as part of the<br>
existing version (v2) of VPN, instead of introducing a new version at<br>
that step?<br>
</blockquote>
<br></span>
Is the Neutron VPN API microversioned? If not, then I suppose your only option is to modify the existing v2 API. These seem to be additive changes, not modifications to existing API calls, in which case they are backwards-compatible (just not discoverable via an API microversion).<span><br></span></blockquote></span><div>I suggest to be done as part of the existing version v2 API . As the api tests are in transition from neutron to neutron-vpnaas repo, we can modify the tests and submit as a one patch<br></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Q3) For the new API additions, do I create a new subclass for the<br>
"interface" that includes all the existing APIs, introduce a new class<br>
that is used together with the existing class, or do I add this to the<br>
existing API?<br>
</blockquote>
<br></span>
Until microversioning is introduced to the Neutron VPN API, it should probably be a change to the existing v2 API.<span><br></span></blockquote></span><div>+1 <br></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Q4) With the final multiple local subnet changes, there will be changes<br>
to the VPN service API (delete subnet_id arg) and IPSec connection API<br>
(delete peer_cidrs arg, and add local_endpoints and peer_endpoints<br>
args). Do we modify the URI so that it calls out v3 (versus v2)? Where<br>
do we do that?<br>
</blockquote>
<br></span>
Hmm, with the backwards-incompatible API changes like the above, your only option is to increment the major version number. The alternative would be to add support for microversioning as a prerequisite to the patch that adds backwards-incompatible changes, and then use a microversion to introduce those changes.<br></blockquote></span><div>Right now, we are beefing up scenario tests for VPN, adding microversioning feature seems better option for me, but open to have reviews from community. <br></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Best,<br>
-jay<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
I'm unsure of the mechanism of increasing the version.<br>
<br>
Thanks in advance for any guidance here on how this should be rolled out...<br>
<br>
Regards,<br>
<br>
Paul Michali (pc_m)<br>
<br>
<br></span>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
</blockquote>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></span></div><br></div></div></div></div></div></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div></div></div>