[neutron] Subnet onboard and changing API definitions in neutron-lib

Slawomir Kaplonski skaplons at redhat.com
Sun Dec 23 20:19:40 UTC 2018


Hi,

Sorry for late answer.
I also think that changing this API definition shouldn’t be a problem if it was never implemented yet.
For me case 2 sounds reasonable and I think that we should make API as simple as it’s possible, so we shouldn’t force users to pass with request data which is not really needed. 

— 
Slawek Kaplonski
Senior software engineer
Red Hat

> Wiadomość napisana przez Miguel Lavalle <miguel at mlavalle.com> w dniu 19.12.2018, o godz. 00:55:
> 
> Hi,
> 
> Based on the proposed Neutron patch (https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1252), clearly the intent of ONBOARD_SUBNETS_SPECS is the creation of new subnets when a subnetpool is created or updated. No ambiguity there. Having said that, I agree with you, it is a cumbersome way to create subnets out of subnetpools that forces the user to issue a additional calls to retrieve the the details of the subnets, since few details are returned in the first call: https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1255. As a consequence, I am in favor of just removing ONBOARD_SUBNETS_SPECS and use the 'onboard_network_subnets' action to onboard existing subnets. As for the API definition sitting a long time in neutron-lib, I don't see that as a problem. The API definition hasn't actually been available to users through any functionality in Neutron, so I don't think we have any
> 
> Thanks for working on this
> 
> Miguel
> 
> On Mon, Dec 10, 2018 at 4:45 PM 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.
> 
> 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]
> 
> 
> 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).
> 
> 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.
> 
> 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
> 
> 
> 




More information about the openstack-discuss mailing list