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@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@1...), 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@1.... 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@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/definit...
[2] https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definit...
[3] https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definit...