From 5eb292c30c26c255f97f7d9a5a25f040c324c9e1 Mon Sep 17 00:00:00 2001
From: zengfagao <zengfa.gao@hp.com>
Date: Sat, 11 Apr 2015 07:15:14 -0700
Subject: [PATCH] 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
---
 neutronclient/common/utils.py                 |  9 +++++++++
 neutronclient/shell.py                        | 15 +++++++++++++++
 neutronclient/tests/unit/test_cli20_subnet.py | 18 +++++++++++++++++-
 neutronclient/tests/unit/test_utils.py        |  5 +++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/neutronclient/common/utils.py b/neutronclient/common/utils.py
index a847b0f..2ddd5a8 100644
--- a/neutronclient/common/utils.py
+++ b/neutronclient/common/utils.py
@@ -19,6 +19,7 @@
 
 import argparse
 import logging
+import netaddr
 import os
 
 from oslo.utils import encodeutils
@@ -171,3 +172,11 @@ def add_boolean_argument(parser, name, **kwargs):
         choices=['True', 'true', 'False', 'false'],
         default=default,
         **kwargs)
+
+
+def is_valid_cidr(cidr):
+    try:
+        netaddr.IPNetwork(cidr)
+        return True
+    except Exception:
+        return False
diff --git a/neutronclient/shell.py b/neutronclient/shell.py
index bf77b4a..bea76cb 100644
--- a/neutronclient/shell.py
+++ b/neutronclient/shell.py
@@ -97,10 +97,25 @@ def run_command(cmd, cmd_parser, sub_argv):
         _argv = sub_argv[:index]
         values_specs = sub_argv[index:]
     known_args, _values_specs = cmd_parser.parse_known_args(_argv)
+    if(isinstance(cmd, subnet.CreateSubnet) and not known_args.cidr):
+        cidr = get_first_valid_cidr(_values_specs)
+        if cidr:
+            known_args.cidr = cidr
+            _values_specs.remove(cidr)
     cmd.values_specs = (index == -1 and _values_specs or values_specs)
     return cmd.run(known_args)
 
 
+def get_first_valid_cidr(value_specs):
+    # Bug 1442771, argparse does not allow optional positional parameter
+    # to be separated from previous positional parameter.
+    # When cidr was separated from network, the value will not be able
+    # to be parsed into known_args, but saved to _values_specs instead.
+    for value in value_specs:
+        if utils.is_valid_cidr(value):
+            return value
+
+
 def env(*_vars, **kwargs):
     """Search for the first defined of possibly many env vars.
 
diff --git a/neutronclient/tests/unit/test_cli20_subnet.py b/neutronclient/tests/unit/test_cli20_subnet.py
index 65cace3..e6a7ea3 100644
--- a/neutronclient/tests/unit/test_cli20_subnet.py
+++ b/neutronclient/tests/unit/test_cli20_subnet.py
@@ -34,7 +34,7 @@ class CLITestV20SubnetJSON(test_cli20.CLITestV20Base):
         name = 'myname'
         myid = 'myid'
         netid = 'netid'
-        cidr = 'cidrvalue'
+        cidr = '10.10.10.0/24'
         gateway = 'gatewayvalue'
         args = ['--gateway', gateway, netid, cidr]
         position_names = ['ip_version', 'network_id', 'cidr', 'gateway_ip']
@@ -42,6 +42,22 @@ class CLITestV20SubnetJSON(test_cli20.CLITestV20Base):
         self._test_create_resource(resource, cmd, name, myid, args,
                                    position_names, position_values)
 
+    def test_create_subnet_network_cidr_seperated(self):
+        # For positional value, network_id and cidr can be separated.
+        """Create subnet: --gateway gateway netid cidr."""
+        resource = 'subnet'
+        cmd = subnet.CreateSubnet(test_cli20.MyApp(sys.stdout), None)
+        name = 'myname'
+        myid = 'myid'
+        netid = 'netid'
+        cidr = '10.10.10.0/24'
+        gateway = 'gatewayvalue'
+        args = [netid, '--gateway', gateway, cidr]
+        position_names = ['ip_version', 'network_id', 'cidr', 'gateway_ip']
+        position_values = [4, netid, cidr, gateway]
+        self._test_create_resource(resource, cmd, name, myid, args,
+                                   position_names, position_values)
+
     def test_create_subnet_with_no_gateway(self):
         """Create subnet: --no-gateway netid cidr."""
         resource = 'subnet'
diff --git a/neutronclient/tests/unit/test_utils.py b/neutronclient/tests/unit/test_utils.py
index a0044d6..a8d3a6e 100644
--- a/neutronclient/tests/unit/test_utils.py
+++ b/neutronclient/tests/unit/test_utils.py
@@ -102,6 +102,11 @@ class TestUtils(testtools.TestCase):
         act = utils.get_item_properties(item, fields, formatters=formatters)
         self.assertEqual(('test_name', 'test_id', 'test', 'pass'), act)
 
+    def test_is_cidr(self):
+        self.assertTrue(utils.is_valid_cidr('10.10.10.0/24'))
+        self.assertFalse(utils.is_valid_cidr('10.10.10..0/24'))
+        self.assertFalse(utils.is_valid_cidr('wrong_cidr_format'))
+
 
 class ImportClassTestCase(testtools.TestCase):
     def test_get_client_class_invalid_version(self):