My two cents.. On Mon, Aug 24, 2015 at 8:48 AM, Jay Pipes <jaypi...@gmail.com> wrote:
> Hi Paul, comments inline... > > On 08/24/2015 07:02 AM, Paul Michali wrote: > >> Hi, >> >> I'm working on the multiple local subnet feature for VPN (RFE >> https://bugs.launchpad.net/neutron/+bug/1459423), with a developer >> reference document detailing the proposed process >> (https://review.openstack.org/#/c/191944/). The plan is to do this in >> two steps. The first is to add new APIs and database support for >> "endpoint groups" (see dev ref for details). The second is to modify the >> IPSec/VPN APIs to make use of the new information (and no longer use >> some older, but equivalent info that is being extended). >> >> I have a few process/procedural questions for the community... >> >> Q1) Should I do this all as one huge commit, as two commits (one for >> endpoint groups and one for modification to support multiple local >> subnets), or multiple (chained) commits (e.g. commit for each API for >> the endpoint groups and for each part of the multiple subnet change)? >> >> My thought (now) is to do this as two commits, with the endpoint groups >> as one, and multiple subnet groups as a second. I started with a commit >> for create API of endpoint (212692), and then did a chained commit for >> delete/show/list (215717), thinking they could be reviewed in pieces, >> but they are not that large and could be easily merged. >> > > My advice would be 2 commits, as you have split them out. > 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. > > Q2) If the two parts are done separately, should the "endpoint group" >> portion, which adds a table and API calls, be done as part of the >> existing version (v2) of VPN, instead of introducing a new version at >> that step? >> > > 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). > 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 > > Q3) For the new API additions, do I create a new subclass for the >> "interface" that includes all the existing APIs, introduce a new class >> that is used together with the existing class, or do I add this to the >> existing API? >> > > Until microversioning is introduced to the Neutron VPN API, it should > probably be a change to the existing v2 API. > +1 > > Q4) With the final multiple local subnet changes, there will be changes >> to the VPN service API (delete subnet_id arg) and IPSec connection API >> (delete peer_cidrs arg, and add local_endpoints and peer_endpoints >> args). Do we modify the URI so that it calls out v3 (versus v2)? Where >> do we do that? >> > > 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. > 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. > > Best, > -jay > > I'm unsure of the mechanism of increasing the version. >> >> Thanks in advance for any guidance here on how this should be rolled >> out... >> >> Regards, >> >> Paul Michali (pc_m) >> >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: >> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev