From a90c824e0407f931b5c45df53103b43aa564de12 Mon Sep 17 00:00:00 2001
From: Richard Theis <rtheis@us.ibm.com>
Date: Tue, 5 Apr 2016 13:27:42 -0500
Subject: [PATCH] Fix router set --route option
Fix the "--route" option on the "os router set" command. The
option did not properly format the new routes to set which
resulted in a "HttpException: Bad Request" error. In addition,
the output for routes was fixed to improve readability and to
align with the "--route" option on the "os router set" command.
Change-Id: I9c514153ec201e2feae32be6dd281771e3298b9c
Closes-Bug: #1564460
---
openstackclient/network/v2/router.py | 26 +++++++++++++++----
openstackclient/tests/network/v2/fakes.py | 6 +++--
.../tests/network/v2/test_router.py | 15 ++++++-----
.../notes/bug-1564460-ab7ad35c02392cb4.yaml | 9 +++++++
4 files changed, 43 insertions(+), 13 deletions(-)
create mode 100644 releasenotes/notes/bug-1564460-ab7ad35c02392cb4.yaml
diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py
index 56630a2303..a32ab5ea79 100644
--- a/openstackclient/network/v2/router.py
+++ b/openstackclient/network/v2/router.py
@@ -34,11 +34,20 @@ def _format_external_gateway_info(info):
return ''
+def _format_routes(routes):
+ # Map the route keys to match --route option.
+ for route in routes:
+ if 'nexthop' in route:
+ route['gateway'] = route.pop('nexthop')
+ return utils.format_list_of_dicts(routes)
+
+
_formatters = {
'admin_state_up': _format_admin_state,
'external_gateway_info': _format_external_gateway_info,
'availability_zones': utils.format_list,
'availability_zone_hints': utils.format_list,
+ 'routes': _format_routes,
}
@@ -67,11 +76,6 @@ def _get_attrs(client_manager, parsed_args):
and parsed_args.availability_zone_hints is not None):
attrs['availability_zone_hints'] = parsed_args.availability_zone_hints
- if 'clear_routes' in parsed_args and parsed_args.clear_routes:
- attrs['routes'] = []
- elif 'routes' in parsed_args and parsed_args.routes is not None:
- attrs['routes'] = parsed_args.routes
-
# "router set" command doesn't support setting project.
if 'project' in parsed_args and parsed_args.project is not None:
identity_client = client_manager.identity
@@ -393,7 +397,19 @@ class SetRouter(command.Command):
client = self.app.client_manager.network
obj = client.find_router(parsed_args.router, ignore_missing=False)
+ # Get the common attributes.
attrs = _get_attrs(self.app.client_manager, parsed_args)
+
+ # Get the route attributes.
+ if parsed_args.clear_routes:
+ attrs['routes'] = []
+ 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.
+ for route in parsed_args.routes:
+ route['nexthop'] = route.pop('gateway')
+ attrs['routes'] = obj.routes + parsed_args.routes
+
if attrs == {}:
msg = "Nothing specified to be set"
raise exceptions.CommandError(msg)
diff --git a/openstackclient/tests/network/v2/fakes.py b/openstackclient/tests/network/v2/fakes.py
index 5fd7ea3b86..73c6b09eb8 100644
--- a/openstackclient/tests/network/v2/fakes.py
+++ b/openstackclient/tests/network/v2/fakes.py
@@ -328,7 +328,7 @@ class FakeRouter(object):
"""Fake one or more routers."""
@staticmethod
- def create_one_router(attrs={}):
+ def create_one_router(attrs=None):
"""Create a fake router.
:param Dictionary attrs:
@@ -337,6 +337,8 @@ class FakeRouter(object):
A FakeResource object, with id, name, admin_state_up,
status, tenant_id
"""
+ attrs = attrs or {}
+
# Set default attributes.
router_attrs = {
'id': 'router-id-' + uuid.uuid4().hex,
@@ -364,7 +366,7 @@ class FakeRouter(object):
return router
@staticmethod
- def create_routers(attrs={}, count=2):
+ def create_routers(attrs=None, count=2):
"""Create multiple fake routers.
:param Dictionary attrs:
diff --git a/openstackclient/tests/network/v2/test_router.py b/openstackclient/tests/network/v2/test_router.py
index e804149867..e3f32e4ae3 100644
--- a/openstackclient/tests/network/v2/test_router.py
+++ b/openstackclient/tests/network/v2/test_router.py
@@ -138,7 +138,7 @@ class TestCreateRouter(TestRouter):
new_router.id,
new_router.name,
new_router.tenant_id,
- new_router.routes,
+ router._format_routes(new_router.routes),
new_router.status,
)
@@ -268,7 +268,7 @@ class TestListRouter(TestRouter):
r = routers[i]
data_long.append(
data[i] + (
- r.routes,
+ router._format_routes(r.routes),
router._format_external_gateway_info(r.external_gateway_info),
osc_utils.format_list(r.availability_zones),
)
@@ -399,7 +399,10 @@ class TestRemoveSubnetFromRouter(TestRouter):
class TestSetRouter(TestRouter):
# The router to set.
- _router = network_fakes.FakeRouter.create_one_router()
+ _default_route = {'destination': '10.20.20.0/24', 'nexthop': '10.20.30.1'}
+ _router = network_fakes.FakeRouter.create_one_router(
+ attrs={'routes': [_default_route]}
+ )
def setUp(self):
super(TestSetRouter, self).setUp()
@@ -491,8 +494,8 @@ class TestSetRouter(TestRouter):
result = self.cmd.take_action(parsed_args)
attrs = {
- 'routes': [{'destination': '10.20.30.0/24',
- 'gateway': '10.20.30.1'}],
+ 'routes': self._router.routes + [{'destination': '10.20.30.0/24',
+ 'nexthop': '10.20.30.1'}],
}
self.network.update_router.assert_called_once_with(
self._router, **attrs)
@@ -572,7 +575,7 @@ class TestShowRouter(TestRouter):
_router.id,
_router.name,
_router.tenant_id,
- _router.routes,
+ router._format_routes(_router.routes),
_router.status,
)
diff --git a/releasenotes/notes/bug-1564460-ab7ad35c02392cb4.yaml b/releasenotes/notes/bug-1564460-ab7ad35c02392cb4.yaml
new file mode 100644
index 0000000000..54b9bdd7bf
--- /dev/null
+++ b/releasenotes/notes/bug-1564460-ab7ad35c02392cb4.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+ - Fixed the ``--route`` option on the ``router set`` command
+ which did not properly format the new routes to set resulting
+ in a ``Bad Request`` error. In addition, the ``router create``,
+ ``router list`` and ``router show`` command output for routes
+ was fixed to improve readability and to align with the
+ ``--route`` option on the ``router set`` command.
+ [Bug `1564460 <https://bugs.launchpad.net/bugs/1564460>`_]