Open Stack

On Mon, 10 Dec 2018 at 23:46, Ryan Tidwell <rtidwell at suse.com> wrote:

> All,
>
> I alluded to some concerns about the current state of subnet onboard at
> the end of today's neutron team meeting. I felt the mailing list was
> probably the best starting point for a discussion, so here it goes :)
>
>
> As I'm dealing with the last loose ends, I'm starting to deal with the
> 'subnets' extension to the subnetpool resource on the API [1]. This has
> me scratching my head at what the intent of this is since there isn't
> much history to work with. When I look at the definition of the subnet
> onboard extension in neutron-lib, I can see two possible "meanings" of
> POST/PUT here:
>
> 1. Create a subnet from this subnetpool using the default prefix length
> of the subnet pool.
>
> 2. Onboard the given subnet into the subnet pool.
>
As mentioned, when I worked on this blueprint, I only thought of this
option. It does not make much sense to use this extension for other steps
(especially as we already have API for 1)

>
> In addition to this ambiguity around the usage of the API, I also have
> concerns that ONBOARD_SUBNET_SPECS as currently defined makes no sense
> for either case.  ONBOARD_SUBNET_SPECS requires that an id, network_id,
> and ip_version be sent on any request made. This seems unnecessary for
> both cases.
>
> Case 1 where we assume that we are using an alternate API to create a
> subnet is problematic in the following ways:
>
> - Specifying an ip_version is unnecessary because it can be inferred
> from the subnet pool
>
> - The definition as written doesn't seem to allow us to respond to the
> user with anything other than a UUID. The user then has to make a second
> call to actually go read the details like CIDR that are going to be
> useful for them going forward.
>
> - More importantly, we already have an API (and corresponding CLI) for
> creating subnets that works well and has much more flexibility than this
> provides. Why duplicate functionality here?
>
>
> Case 2 where we assume that we are onboarding a subnet is problematic in
> the following ways:
>
> - Specifying network_id and ip_version are unnecessary. These values can
> be read right off the subnet (which we would need to read for onboarding
> anyway), all that needs to be passed is the subnet UUID.
>
> - Again, we would be duplicating functionality here because we have
> already defined an API for onboarding a subnet in the ACTION_MAP [2]
>
Indeed, re-reading the blueprint, we "just" need to act on a subnet (from
its id) to add it to a subnet pool
So extra parameters look unnecessary at best

>
>
> My intent is to figure out where to go from here to make this better
> and/or alleviate the confusion on my part so this feature can be wrapped
> up. Here is what I propose:
>
> Because subnet onboard is still incomplete and we are not claiming
> support for it yet, I am proposing we simply remove the 'subnets'
> extension to the subnetpools resource [3]. This simplifies the API and
> resolves the concerns I expressed above. It also allows us to quickly
> finish up subnet onboard without losing any of the desired
> functionality, namely the ability to move ("onboard") an existing subnet
> into a subnet pool (and by extension and address scope).
>
https://review.openstack.org/#/c/625936/ (for further discussion)

>
> The reason I am looking for input here is that it isn't clear to me
> whether the approach I'm suggesting is acceptable to the team given our
> policies around changing API definitions in neutron-lib. I'm not aware
> of a situation where we have had an unreleased feature and have
> discovered we don't like the API definition in neutron-lib (which has
> sat for quite a while unimplemented) and want to change it. I'm just not
> aware of any precedent for this situation, so I'm hoping the team has
> some thoughts on how best to move forward.
>
We already did a change to this API definition recently with the default
value change in change I6c0b5aa784a380502b9e5fd062dacd10a95b4cbf so I would
say we can do the same here (fixing API before first implementation gets
out)

>
> For reference, the current review for subnet onboard is
> https://review.openstack.org/#/c/348080/. Any thoughts on this topic
> would be greatly appreciated by me, and hopefully this discussion can be
> useful to a broad audience going forward.
>
>
> -Ryan
>
>
> [1]
>
> https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L32
>
> [2]
>
> https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L58
>
> [3]
>
> https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L41
>
>
>
>

-- 
Bernard Cafarelli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20181218/424e446d/attachment-0001.html>