Open Stack

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.

> 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).

> 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.

> 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.

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-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>