openstack/python-openstackclient
Code Issues Proposed changes

Merge "Introduce overwrite functionality in `osc router set`"

Browse Source
This commit is contained in:
Jenkins 2016-12-21 22:09:50 +00:00 committed by Gerrit Code Review
commit 5d62981beb
4 changed files with 60 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/command-objects/router.rst
View File

@ -259,7 +259,9 @@ Set router properties
.. option:: --no-route
Clear routes associated with the router
Clear routes associated with the router.
Specify both --route and --no-route to overwrite
current value of route.
.. option:: --ha

27
openstackclient/network/v2/router.py
View File

@ -98,8 +98,6 @@ def _get_attrs(client_manager, parsed_args):
).id
attrs['tenant_id'] = project_id
# TODO(tangchen): Support getting 'external_gateway_info' property.
return attrs
@ -464,7 +462,8 @@ class SetRouter(command.Command):
help=_("Set router to centralized mode (disabled router only)")
)
routes_group = parser.add_mutually_exclusive_group()
routes_group.add_argument(
# ToDo(Reedip):Remove mutual exclusiveness once clear-routes is removed
parser.add_argument(
'--route',
metavar='destination=<subnet>,gateway=<ip-address>',
action=parseractions.MultiKeyValueAction,
@ -479,7 +478,9 @@ class SetRouter(command.Command):
routes_group.add_argument(
'--no-route',
action='store_true',
help=_("Clear routes associated with the router")
help=_("Clear routes associated with the router. "
"Specify both --route and --no-route to overwrite "
"current value of route.")
)
routes_group.add_argument(
'--clear-routes',
@ -539,20 +540,22 @@ class SetRouter(command.Command):
attrs['ha'] = True
elif parsed_args.no_ha:
attrs['ha'] = False
if parsed_args.no_route:
attrs['routes'] = []
elif parsed_args.clear_routes:
attrs['routes'] = []
if parsed_args.clear_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.
if parsed_args.routes is not None:
for route in parsed_args.routes:
route['nexthop'] = route.pop('gateway')
attrs['routes'] = obj.routes + parsed_args.routes
attrs['routes'] = parsed_args.routes
if not (parsed_args.no_route or parsed_args.clear_routes):
# Map the route keys and append to the current routes.
# The REST API will handle route validation and duplicates.
attrs['routes'] += obj.routes
elif parsed_args.no_route or parsed_args.clear_routes:
attrs['routes'] = []
if (parsed_args.disable_snat or parsed_args.enable_snat or
parsed_args.fixed_ip) and not parsed_args.external_gateway:
msg = (_("You must specify '--external-gateway' in order"

50
openstackclient/tests/unit/network/v2/test_router.py
View File

@ -704,10 +704,10 @@ class TestSetRouter(TestRouter):
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
routes = [{'destination': '10.20.30.0/24',
'nexthop': '10.20.30.1'}]
attrs = {
'routes': self._router.routes + [{'destination': '10.20.30.0/24',
'nexthop': '10.20.30.1'}],
'routes': routes + self._router.routes
}
self.network.update_router.assert_called_once_with(
self._router, **attrs)
@ -733,21 +733,31 @@ class TestSetRouter(TestRouter):
self._router, **attrs)
self.assertIsNone(result)
def test_set_route_no_route(self):
def test_set_route_overwrite_route(self):
_testrouter = network_fakes.FakeRouter.create_one_router(
{'routes': [{"destination": "10.0.0.2",
"nexthop": "1.1.1.1"}]})
self.network.find_router = mock.Mock(return_value=_testrouter)
arglist = [
self._router.name,
_testrouter.name,
'--route', 'destination=10.20.30.0/24,gateway=10.20.30.1',
'--no-route',
]
verifylist = [
('router', self._router.name),
('router', _testrouter.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)
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
attrs = {
'routes': [{'destination': '10.20.30.0/24',
'nexthop': '10.20.30.1'}]
}
self.network.update_router.assert_called_once_with(
_testrouter, **attrs)
self.assertIsNone(result)
def test_set_clear_routes(self):
arglist = [
@ -769,21 +779,31 @@ class TestSetRouter(TestRouter):
self._router, **attrs)
self.assertIsNone(result)
def test_set_route_clear_routes(self):
def test_overwrite_route_clear_routes(self):
_testrouter = network_fakes.FakeRouter.create_one_router(
{'routes': [{"destination": "10.0.0.2",
"nexthop": "1.1.1.1"}]})
self.network.find_router = mock.Mock(return_value=_testrouter)
arglist = [
self._router.name,
_testrouter.name,
'--route', 'destination=10.20.30.0/24,gateway=10.20.30.1',
'--clear-routes',
]
verifylist = [
('router', self._router.name),
('router', _testrouter.name),
('routes', [{'destination': '10.20.30.0/24',
'gateway': '10.20.30.1'}]),
('clear_routes', True),
]
self.assertRaises(tests_utils.ParserException, self.check_parser,
self.cmd, arglist, verifylist)
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
attrs = {
'routes': [{'destination': '10.20.30.0/24',
'nexthop': '10.20.30.1'}]
}
self.network.update_router.assert_called_once_with(
_testrouter, **attrs)
self.assertIsNone(result)
def test_set_nothing(self):
arglist = [

7
releasenotes/notes/add-overwrite-option-to-router-7c50c8031dab6bae.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
Add support to overwrite routes in a router instance, using ``--router``
and ``--no-router`` option in ``osc router set``.
[ Blueprint `allow-overwrite-set-options <https://blueprints.launchpad.net/python-openstackclient/+spec/allow-overwrite-set-options>`_]