2.4.0 broke existing subnet create calls

[Kyle Mestery]

It appears that the 2.4.0 release broke subnet create calls.

Looking at this review [1], you can see a subnet create call like this:

neutron --os-tenant-name accounts-test-$i --os-password pass --os-username accounts-test--$i \
 subnet-create network-$i --name subnet-$i --gateway 10.0.$i.1 10.0.$i.0/24

Looking at the logs for this [2], you can see a failure (copied below):

2015-04-10 17:34:50.665 | ++ neutron --os-tenant-name accounts-test-1 --os-password pass --os-username accounts-test--1 subnet-create network-1 --name subnet-1 --gateway 10.0.1.1 10.0.1.0/24
2015-04-10 17:34:51.467 | Invalid values_specs 10.0.1.0/24

[1] https://review.openstack.org/#/c/158511/
[2] http://logs.openstack.org/11/158511/30/check/check-tempest-dsvm-neutron-full/80d925a/logs/devstacklog.txt.gz#_2015-04-10_17_34_50_665

[Kyle Mestery (mestery)]

We're thinking this review may be the culprit:

https://review.openstack.org/#/c/165910/6

[Zengfa Gao (zfgao)]

I will look at this issue soon.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/172548

[Zengfa Gao (zfgao)]

The one works: neutron subnet-create network1 10.10.1.0/24 --name subnet1

In the old way, both name and CIDR is positional arguments, you can separate them, argparse can still parse it.
With subnetpool, CIDR is optional positional arguments, when you have another parameter in between, it cannot parse it right.
Looking to see if we have better solution.

[Zengfa Gao (zfgao)]

For python argparse, it has the conception of positional parameter such as 'NAME CIDR'.
If both are required, then they can be separated.
If later is optional positional parameter, those two need to be together. I didn't find a good way to solve this problem yet.

With subnetpool, CIDR is optional positional parameter for subnet-create.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/172646

[Carl Baldwin (carl-baldwin)]

I see now how this happened. By making cidr an optional argument, it changed the way that positional arguments were parsed. This was not a concern when designing the change at the API level because a REST API doesn't have the concept of a positional argument. This is a concern only because the client allows arguments to be specified by position. This does indeed make things more fragile.

I think the proposed fix which would use the cidr argument for either cidr or prefixlen exposes too much ugliness to the end user. I find it confusing to have such a hybrid argument. It is a hack.

Unfortunately, given the constraints of argparse, I can't think of a solution that isn't a hack. But, I think we could come up with something that ends up presenting a nice clean interface to the end user.

My thought was to maybe do a little pre-processing of the args before argparse gets called. We could detect the ambiguity and edit args enough to provide argparse with a hint based on whether a cidr or a integer is being passed. Then, call argparse on the result. Thoughts?

[Zengfa Gao (zfgao)]

New proposal was uploaded to find cidr from _values_specs for SubnetCreate.
https://review.openstack.org/#/c/172646/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/172646
Committed: https://git.openstack.org/cgit/openstack/python-neutronclient/commit/?id=5eb292c30c26c255f97f7d9a5a25f040c324c9e1
Submitter: Jenkins
Branch: master

commit 5eb292c30c26c255f97f7d9a5a25f040c324c9e1
Author: zengfagao <email address hidden>
Date: Sat Apr 11 07:15:14 2015 -0700

    Fix Python client library for Neutron

    To create a subnet, we used to require positional cidr value.
    With subnetpool, cidr is now optional value. With optional
    positional cidr, the cidr value cannot be parsed into known_args
    if it is separated from network value.

    To fix the problem, we need to find cidr value from value_specs,
    and use it if we find it. Cidr is optional, may not exist.

    Change-Id: Ic4bccf4e7384cc891cef71fb7ce3b52fbf997345
    Closes-Bug:#1442771

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/172548
Committed: https://git.openstack.org/cgit/openstack/python-neutronclient/commit/?id=6190a721030094d8eb9016fb85cb56e7d451b157
Submitter: Jenkins
Branch: master

commit 6190a721030094d8eb9016fb85cb56e7d451b157
Author: Matthew Treinish <email address hidden>
Date: Fri Apr 10 15:41:44 2015 -0400

    Add functional test for subnet create

    This test adds a functional test to verify a subnet-create command
    with the arg order used in the docs. python-neutronclient introduced
    a regression which broke the usage of this order. This test will
    prevent this from happening in the future.

    Change-Id: If7e4211a4cbf33bc87a1304553ad3dc9c89346c4
    Related-Bug: #1442771