From 25104c7d4a23092e6173bb76171d2d7ee6c4694b Mon Sep 17 00:00:00 2001 From: Reedip Date: Wed, 16 Nov 2016 15:56:57 +0530 Subject: [PATCH] Introduce overwrite functionality in ``osc router set`` The overwrite functionality allows user to overwrite the routes of a specific router. Change-Id: I8d3cfe5cab2ffbfa046371c3adcd2cf446c91cbc partially-implements: blueprint allow-overwrite-set-options --- doc/source/command-objects/router.rst | 4 +- openstackclient/network/v2/router.py | 27 +++++----- .../tests/unit/network/v2/test_router.py | 50 +++++++++++++------ ...ite-option-to-router-7c50c8031dab6bae.yaml | 7 +++ 4 files changed, 60 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/add-overwrite-option-to-router-7c50c8031dab6bae.yaml diff --git a/doc/source/command-objects/router.rst b/doc/source/command-objects/router.rst index 16a8258dfd..d2190ba4b4 100644 --- a/doc/source/command-objects/router.rst +++ b/doc/source/command-objects/router.rst @@ -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 diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py index fea294dadb..4d779c3767 100644 --- a/openstackclient/network/v2/router.py +++ b/openstackclient/network/v2/router.py @@ -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=,gateway=', 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" diff --git a/openstackclient/tests/unit/network/v2/test_router.py b/openstackclient/tests/unit/network/v2/test_router.py index 9183cb6364..faca7479c4 100644 --- a/openstackclient/tests/unit/network/v2/test_router.py +++ b/openstackclient/tests/unit/network/v2/test_router.py @@ -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 = [ diff --git a/releasenotes/notes/add-overwrite-option-to-router-7c50c8031dab6bae.yaml b/releasenotes/notes/add-overwrite-option-to-router-7c50c8031dab6bae.yaml new file mode 100644 index 0000000000..ac83b7e84e --- /dev/null +++ b/releasenotes/notes/add-overwrite-option-to-router-7c50c8031dab6bae.yaml @@ -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 `_] +