From bc93ebfe5c0fa4d29b79fa3fd93ec603425997ea Mon Sep 17 00:00:00 2001
From: Hideki Saito <saito@fgrep.org>
Date: Sat, 14 May 2016 20:48:43 +0900
Subject: [PATCH] Added --no-route to the router set command

Deprecated --clear-routes

Closes-Bug #1565034

Change-Id: I4a8975edc026aecd2a362fd1929c984cfab8ade6
---
 doc/source/command-objects/router.rst         |  4 +--
 openstackclient/network/v2/router.py          | 20 +++++++++--
 .../tests/network/v2/test_router.py           | 36 +++++++++++++++++++
 .../notes/bug-1565034-dd404bfb42d7778d.yaml   |  5 +++
 4 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/bug-1565034-dd404bfb42d7778d.yaml

diff --git a/doc/source/command-objects/router.rst b/doc/source/command-objects/router.rst
index 1bb9341ebf..9ca7661ede 100644
--- a/doc/source/command-objects/router.rst
+++ b/doc/source/command-objects/router.rst
@@ -186,7 +186,7 @@ Set router properties
         [--name <name>]
         [--enable | --disable]
         [--distributed | --centralized]
-        [--route destination=<subnet>,gateway=<ip-address> | --clear-routes]
+        [--route destination=<subnet>,gateway=<ip-address> | --no-route]
         <router>
 
 .. option:: --name <name>
@@ -216,7 +216,7 @@ Set router properties
     gateway: nexthop IP address
     (repeat option to set multiple routes)
 
-.. option:: --clear-routes
+.. option:: --no-route
 
     Clear routes associated with the router
 
diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py
index a32ab5ea79..e479eee3a0 100644
--- a/openstackclient/network/v2/router.py
+++ b/openstackclient/network/v2/router.py
@@ -13,7 +13,9 @@
 
 """Router action implementations"""
 
+import argparse
 import json
+import logging
 
 from openstackclient.common import command
 from openstackclient.common import exceptions
@@ -23,6 +25,9 @@ from openstackclient.i18n import _
 from openstackclient.identity import common as identity_common
 
 
+LOG = logging.getLogger(__name__)
+
+
 def _format_admin_state(state):
     return 'UP' if state else 'DOWN'
 
@@ -379,10 +384,15 @@ class SetRouter(command.Command):
                    "(repeat option to set multiple routes)")
         )
         routes_group.add_argument(
-            '--clear-routes',
+            '--no-route',
             action='store_true',
             help=_("Clear routes associated with the router")
         )
+        routes_group.add_argument(
+            '--clear-routes',
+            action='store_true',
+            help=argparse.SUPPRESS,
+        )
 
         # TODO(tangchen): Support setting 'ha' property in 'router set'
         # command. It appears that changing the ha state is supported by
@@ -401,8 +411,14 @@ class SetRouter(command.Command):
         attrs = _get_attrs(self.app.client_manager, parsed_args)
 
         # Get the route attributes.
-        if parsed_args.clear_routes:
+        if parsed_args.no_route:
             attrs['routes'] = []
+        elif parsed_args.clear_routes:
+            attrs['routes'] = []
+            LOG.warning(_(
+                'The --clear-routes option is deprecated, '
+                'please use --no-route instead.'
+            ))
         elif parsed_args.routes is not None:
             # Map the route keys and append to the current routes.
             # The REST API will handle route validation and duplicates.
diff --git a/openstackclient/tests/network/v2/test_router.py b/openstackclient/tests/network/v2/test_router.py
index 655e86c95e..99b41d2dc4 100644
--- a/openstackclient/tests/network/v2/test_router.py
+++ b/openstackclient/tests/network/v2/test_router.py
@@ -495,6 +495,42 @@ class TestSetRouter(TestRouter):
             self._router, **attrs)
         self.assertIsNone(result)
 
+    def test_set_no_route(self):
+        arglist = [
+            self._router.name,
+            '--no-route',
+        ]
+        verifylist = [
+            ('router', self._router.name),
+            ('no_route', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        attrs = {
+            'routes': [],
+        }
+        self.network.update_router.assert_called_once_with(
+            self._router, **attrs)
+        self.assertIsNone(result)
+
+    def test_set_route_no_route(self):
+        arglist = [
+            self._router.name,
+            '--route', 'destination=10.20.30.0/24,gateway=10.20.30.1',
+            '--no-route',
+        ]
+        verifylist = [
+            ('router', self._router.name),
+            ('routes', [{'destination': '10.20.30.0/24',
+                         'gateway': '10.20.30.1'}]),
+            ('no_route', True),
+        ]
+
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
     def test_set_clear_routes(self):
         arglist = [
             self._router.name,
diff --git a/releasenotes/notes/bug-1565034-dd404bfb42d7778d.yaml b/releasenotes/notes/bug-1565034-dd404bfb42d7778d.yaml
new file mode 100644
index 0000000000..e5ff38e8b2
--- /dev/null
+++ b/releasenotes/notes/bug-1565034-dd404bfb42d7778d.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Added ``--no-route`` to the ``router set`` command.
+    Deprecated ``--clear-routes``.
+    [Bug `1565034 <https://bugs.launchpad.net/bugs/1565034>`_]