<div dir="ltr">My two cents..<br><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">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 class=""><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 class=""><br></span></blockquote><div><br></div><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><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<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 class=""><br></span></blockquote><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><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<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 class=""><br></span></blockquote><div>+1 <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<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><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><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 class="">
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></div><br></div></div></div></div></div></div>