On Mon, 10 Dec 2018 at 23:46, Ryan Tidwell <rtidwell@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