<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Hi,</div><div><br></div><div>Based on the proposed Neutron patch (<a href="https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1252">https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1252</a>), 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: <a href="https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1255">https://review.openstack.org/#/c/348080/36/neutron/db/db_base_plugin_v2.py@1255</a>. 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<br></div><div><br></div><div>Thanks for working on this</div><div><br></div><div>Miguel<br></div></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 10, 2018 at 4:45 PM Ryan Tidwell <<a href="mailto:rtidwell@suse.com">rtidwell@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">All,<br>
<br>
I alluded to some concerns about the current state of subnet onboard at<br>
the end of today's neutron team meeting. I felt the mailing list was<br>
probably the best starting point for a discussion, so here it goes :)<br>
<br>
<br>
As I'm dealing with the last loose ends, I'm starting to deal with the<br>
'subnets' extension to the subnetpool resource on the API [1]. This has<br>
me scratching my head at what the intent of this is since there isn't<br>
much history to work with. When I look at the definition of the subnet<br>
onboard extension in neutron-lib, I can see two possible "meanings" of<br>
POST/PUT here:<br>
<br>
1. Create a subnet from this subnetpool using the default prefix length<br>
of the subnet pool.<br>
<br>
2. Onboard the given subnet into the subnet pool.<br>
<br>
In addition to this ambiguity around the usage of the API, I also have<br>
concerns that ONBOARD_SUBNET_SPECS as currently defined makes no sense<br>
for either case.  ONBOARD_SUBNET_SPECS requires that an id, network_id,<br>
and ip_version be sent on any request made. This seems unnecessary for<br>
both cases.<br>
<br>
Case 1 where we assume that we are using an alternate API to create a<br>
subnet is problematic in the following ways:<br>
<br>
- Specifying an ip_version is unnecessary because it can be inferred<br>
from the subnet pool<br>
<br>
- The definition as written doesn't seem to allow us to respond to the<br>
user with anything other than a UUID. The user then has to make a second<br>
call to actually go read the details like CIDR that are going to be<br>
useful for them going forward.<br>
<br>
- More importantly, we already have an API (and corresponding CLI) for<br>
creating subnets that works well and has much more flexibility than this<br>
provides. Why duplicate functionality here?<br>
<br>
<br>
Case 2 where we assume that we are onboarding a subnet is problematic in<br>
the following ways:<br>
<br>
- Specifying network_id and ip_version are unnecessary. These values can<br>
be read right off the subnet (which we would need to read for onboarding<br>
anyway), all that needs to be passed is the subnet UUID.<br>
<br>
- Again, we would be duplicating functionality here because we have<br>
already defined an API for onboarding a subnet in the ACTION_MAP [2]<br>
<br>
<br>
My intent is to figure out where to go from here to make this better<br>
and/or alleviate the confusion on my part so this feature can be wrapped<br>
up. Here is what I propose:<br>
<br>
Because subnet onboard is still incomplete and we are not claiming<br>
support for it yet, I am proposing we simply remove the 'subnets'<br>
extension to the subnetpools resource [3]. This simplifies the API and<br>
resolves the concerns I expressed above. It also allows us to quickly<br>
finish up subnet onboard without losing any of the desired<br>
functionality, namely the ability to move ("onboard") an existing subnet<br>
into a subnet pool (and by extension and address scope).<br>
<br>
The reason I am looking for input here is that it isn't clear to me<br>
whether the approach I'm suggesting is acceptable to the team given our<br>
policies around changing API definitions in neutron-lib. I'm not aware<br>
of a situation where we have had an unreleased feature and have<br>
discovered we don't like the API definition in neutron-lib (which has<br>
sat for quite a while unimplemented) and want to change it. I'm just not<br>
aware of any precedent for this situation, so I'm hoping the team has<br>
some thoughts on how best to move forward.<br>
<br>
For reference, the current review for subnet onboard is<br>
<a href="https://review.openstack.org/#/c/348080/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/348080/</a>. Any thoughts on this topic<br>
would be greatly appreciated by me, and hopefully this discussion can be<br>
useful to a broad audience going forward.<br>
<br>
<br>
-Ryan<br>
<br>
<br>
[1]<br>
<a href="https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L32" rel="noreferrer" target="_blank">https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L32</a><br>
<br>
[2]<br>
<a href="https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L58" rel="noreferrer" target="_blank">https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L58</a><br>
<br>
[3]<br>
<a href="https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L41" rel="noreferrer" target="_blank">https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/subnet_onboard.py#L41</a><br>
<br>
<br>
<br>
</blockquote></div>