Network L3 Router Commands for OSC

Implements: blueprint network-l3-commands

Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
Change-Id: Ia24d76227e164062e89a74c1621b8acb830b26cf

6 files changed, 396 insertions, 50 deletions

diff --git a/openstackclient/network/v2/network_agent.py b/openstackclient/network/v2/network_agent.py
index 1334f77e..ed4970a4 100644
--- a/openstackclient/network/v2/network_agent.py
+++ b/openstackclient/network/v2/network_agent.py
@@ -26,10 +26,16 @@ from openstackclient.network import sdk_utils
 LOG = logging.getLogger(__name__)
 
 
+def _format_alive(alive):
+    return ":-)" if alive else "XXX"
+
+
 def _format_admin_state(state):
     return 'UP' if state else 'DOWN'
 
 _formatters = {
+    'is_alive': _format_alive,
+    'alive': _format_alive,
     'admin_state_up': _format_admin_state,
     'is_admin_state_up': _format_admin_state,
     'configurations': utils.format_dict,
@@ -60,7 +66,7 @@ class AddNetworkToAgent(command.Command):
         parser.add_argument(
             'network',
             metavar='<network>',
-            help=_('Network to be added to an agent (ID or name)'))
+            help=_('Network to be added to an agent (name or ID)'))
 
         return parser
 
@@ -78,6 +84,37 @@ class AddNetworkToAgent(command.Command):
                 exceptions.CommandError(msg)
 
 
+class AddRouterToAgent(command.Command):
+    _description = _("Add router to an agent")
+
+    def get_parser(self, prog_name):
+        parser = super(AddRouterToAgent, self).get_parser(prog_name)
+        parser.add_argument(
+            '--l3',
+            action='store_true',
+            help=_('Add router to an L3 agent')
+        )
+        parser.add_argument(
+            'agent_id',
+            metavar='<agent-id>',
+            help=_("Agent to which a router is added (ID only)")
+        )
+        parser.add_argument(
+            'router',
+            metavar='<router>',
+            help=_("Router to be added to an agent (name or ID)")
+        )
+
+        return parser
+
+    def take_action(self, parsed_args):
+        client = self.app.client_manager.network
+        agent = client.get_agent(parsed_args.agent_id)
+        router = client.find_router(parsed_args.router, ignore_missing=False)
+        if parsed_args.l3:
+            client.add_router_to_agent(agent, router)
+
+
 class DeleteNetworkAgent(command.Command):
     _description = _("Delete network agent(s)")
 
@@ -135,11 +172,24 @@ class ListNetworkAgent(command.Lister):
             metavar='<host>',
             help=_("List only agents running on the specified host")
         )
-        parser.add_argument(
+        agent_type_group = parser.add_mutually_exclusive_group()
+        agent_type_group.add_argument(
             '--network',
             metavar='<network>',
             help=_('List agents hosting a network (name or ID)')
         )
+        agent_type_group.add_argument(
+            '--router',
+            metavar='<router>',
+            help=_('List agents hosting this router (name or ID)')
+        )
+        parser.add_argument(
+            '--long',
+            action='store_true',
+            default=False,
+            help=_("List additional fields in output")
+        )
+
         return parser
 
     def take_action(self, parsed_args):
@@ -178,28 +228,18 @@ class ListNetworkAgent(command.Lister):
         }
 
         filters = {}
+
         if parsed_args.network is not None:
-            columns = (
-                'id',
-                'host',
-                'is_admin_state_up',
-                'is_alive',
-            )
-            column_headers = (
-                'ID',
-                'Host',
-                'Admin State Up',
-                'Alive',
-            )
             network = client.find_network(
                 parsed_args.network, ignore_missing=False)
             data = client.network_hosting_dhcp_agents(network)
-
-            return (column_headers,
-                    (utils.get_item_properties(
-                        s, columns,
-                        formatters=_formatters,
-                    ) for s in data))
+        elif parsed_args.router is not None:
+            if parsed_args.long:
+                columns += ('ha_state',)
+                column_headers += ('HA State',)
+            router = client.find_router(parsed_args.router,
+                                        ignore_missing=False)
+            data = client.routers_hosting_l3_agents(router)
         else:
             if parsed_args.agent_type is not None:
                 filters['agent_type'] = key_value[parsed_args.agent_type]
@@ -207,10 +247,10 @@ class ListNetworkAgent(command.Lister):
                 filters['host'] = parsed_args.host
 
             data = client.agents(**filters)
-            return (column_headers,
-                    (utils.get_item_properties(
-                        s, columns, formatters=_formatters,
-                    ) for s in data))
+        return (column_headers,
+                (utils.get_item_properties(
+                    s, columns, formatters=_formatters,
+                ) for s in data))
 
 
 class RemoveNetworkFromAgent(command.Command):
@@ -229,7 +269,7 @@ class RemoveNetworkFromAgent(command.Command):
         parser.add_argument(
             'network',
             metavar='<network>',
-            help=_('Network to be removed from an agent (ID or name)'))
+            help=_('Network to be removed from an agent (name or ID)'))
         return parser
 
     def take_action(self, parsed_args):
@@ -246,6 +286,37 @@ class RemoveNetworkFromAgent(command.Command):
                 exceptions.CommandError(msg)
 
 
+class RemoveRouterFromAgent(command.Command):
+    _description = _("Remove router from an agent")
+
+    def get_parser(self, prog_name):
+        parser = super(RemoveRouterFromAgent, self).get_parser(prog_name)
+        parser.add_argument(
+            '--l3',
+            action='store_true',
+            help=_('Remove router from an L3 agent')
+        )
+        parser.add_argument(
+            'agent_id',
+            metavar='<agent-id>',
+            help=_("Agent from which router will be removed (ID only)")
+        )
+        parser.add_argument(
+            'router',
+            metavar='<router>',
+            help=_("Router to be removed from an agent (name or ID)")
+        )
+
+        return parser
+
+    def take_action(self, parsed_args):
+        client = self.app.client_manager.network
+        agent = client.get_agent(parsed_args.agent_id)
+        router = client.find_router(parsed_args.router, ignore_missing=False)
+        if parsed_args.l3:
+            client.remove_router_from_agent(agent, router)
+
+
 # TODO(huanxuan): Use the SDK resource mapped attribute names once the
 # OSC minimum requirements include SDK 1.0.
 class SetNetworkAgent(command.Command):
diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py
index 0da91baa..8db0c439 100644
--- a/openstackclient/network/v2/router.py
+++ b/openstackclient/network/v2/router.py
@@ -305,11 +305,18 @@ class ListRouter(command.Lister):
             help=_("List routers according to their project (name or ID)")
         )
         identity_common.add_project_domain_option_to_parser(parser)
+        parser.add_argument(
+            '--agent',
+            metavar='<agent-id>',
+            help=_("List routers hosted by an agent (ID only)")
+        )
+
         return parser
 
     def take_action(self, parsed_args):
         identity_client = self.app.client_manager.identity
         client = self.app.client_manager.network
+
         columns = (
             'id',
             'name',
@@ -349,6 +356,16 @@ class ListRouter(command.Lister):
             ).id
             args['tenant_id'] = project_id
             args['project_id'] = project_id
+
+        if parsed_args.agent is not None:
+            agent = client.get_agent(parsed_args.agent)
+            data = client.agent_hosted_routers(agent)
+            # NOTE: Networking API does not support filtering by parameters,
+            # so we need filtering in the client side.
+            data = [d for d in data if self._filter_match(d, args)]
+        else:
+            data = client.routers(**args)
+
         if parsed_args.long:
             columns = columns + (
                 'routes',
@@ -368,13 +385,26 @@ class ListRouter(command.Lister):
                     'Availability zones',
                 )
 
-        data = client.routers(**args)
         return (column_headers,
                 (utils.get_item_properties(
                     s, columns,
                     formatters=_formatters,
                 ) for s in data))
 
+    @staticmethod
+    def _filter_match(data, conditions):
+        for key, value in conditions.items():
+            try:
+                if getattr(data, key) != value:
+                    return False
+            except AttributeError:
+                # Some filter attributes like tenant_id or admin_state_up
+                # are backward compatibility in older OpenStack SDK support.
+                # They does not exist in the latest release.
+                # In this case we just skip checking such filter condition.
+                continue
+        return True
+
 
 class RemovePortFromRouter(command.Command):
     _description = _("Remove a port from a router")
diff --git a/openstackclient/tests/functional/network/v2/test_network_agent.py b/openstackclient/tests/functional/network/v2/test_network_agent.py
index 16487955..0c74ea1d 100644
--- a/openstackclient/tests/functional/network/v2/test_network_agent.py
+++ b/openstackclient/tests/functional/network/v2/test_network_agent.py
@@ -137,3 +137,39 @@ class NetworkAgentListTests(common.NetworkTests):
         self.assertIn(
             agent_id, col_name
         )
+
+    def test_network_agent_list_routers(self):
+        """Add agent to router, list agents on router, delete."""
+        name = uuid.uuid4().hex
+        cmd_output = json.loads(self.openstack(
+            'router create -f json ' + name))
+
+        self.addCleanup(self.openstack, 'router delete ' + name)
+        # Get router ID
+        router_id = cmd_output['id']
+        # Get l3 agent id
+        cmd_output = json.loads(self.openstack(
+            'network agent list -f json --agent-type l3'))
+
+        # Check at least one L3 agent is included in the response.
+        self.assertTrue(cmd_output)
+        agent_id = cmd_output[0]['ID']
+
+        # Add router to agent
+        self.openstack(
+            'network agent add router --l3 ' + agent_id + ' ' + router_id)
+
+        # Test router list --agent
+        cmd_output = json.loads(self.openstack(
+            'network agent list -f json --router ' + router_id))
+
+        agent_ids = [x['ID'] for x in cmd_output]
+        self.assertIn(agent_id, agent_ids)
+
+        # Remove router from agent
+        self.openstack(
+            'network agent remove router --l3 ' + agent_id + ' ' + router_id)
+        cmd_output = json.loads(self.openstack(
+            'network agent list -f json --router ' + router_id))
+        agent_ids = [x['ID'] for x in cmd_output]
+        self.assertNotIn(agent_id, agent_ids)
diff --git a/openstackclient/tests/functional/network/v2/test_router.py b/openstackclient/tests/functional/network/v2/test_router.py
index 313feefc..2e5cb5ef 100644
--- a/openstackclient/tests/functional/network/v2/test_router.py
+++ b/openstackclient/tests/functional/network/v2/test_router.py
@@ -151,6 +151,40 @@ class RouterTests(common.NetworkTests):
         self.assertIn(name1, names)
         self.assertIn(name2, names)
 
+    def test_router_list_l3_agent(self):
+        """Tests create router, add l3 agent, list, delete"""
+        name = uuid.uuid4().hex
+        cmd_output = json.loads(self.openstack(
+            'router create -f json ' + name))
+
+        self.addCleanup(self.openstack, 'router delete ' + name)
+        # Get router ID
+        router_id = cmd_output['id']
+        # Get l3 agent id
+        cmd_output = json.loads(self.openstack(
+            'network agent list -f json --agent-type l3'))
+
+        # Check at least one L3 agent is included in the response.
+        self.assertTrue(cmd_output)
+        agent_id = cmd_output[0]['ID']
+
+        # Add router to agent
+        self.openstack(
+            'network agent add router --l3 ' + agent_id + ' ' + router_id)
+
+        cmd_output = json.loads(self.openstack(
+            'router list -f json --agent ' + agent_id))
+        router_ids = [x['ID'] for x in cmd_output]
+        self.assertIn(router_id, router_ids)
+
+        # Remove router from agent
+        self.openstack(
+            'network agent remove router --l3 ' + agent_id + ' ' + router_id)
+        cmd_output = json.loads(self.openstack(
+            'router list -f json --agent ' + agent_id))
+        router_ids = [x['ID'] for x in cmd_output]
+        self.assertNotIn(router_id, router_ids)
+
     def test_router_set_show_unset(self):
         """Tests create router, set, unset, show"""
 
diff --git a/openstackclient/tests/unit/network/v2/test_network_agent.py b/openstackclient/tests/unit/network/v2/test_network_agent.py
index 9bb3f090..12e40cdb 100644
--- a/openstackclient/tests/unit/network/v2/test_network_agent.py
+++ b/openstackclient/tests/unit/network/v2/test_network_agent.py
@@ -73,6 +73,46 @@ class TestAddNetworkToAgent(TestNetworkAgent):
             self.agent, self.net)
 
 
+class TestAddRouterAgent(TestNetworkAgent):
+
+    _router = network_fakes.FakeRouter.create_one_router()
+    _agent = network_fakes.FakeNetworkAgent.create_one_network_agent()
+
+    def setUp(self):
+        super(TestAddRouterAgent, self).setUp()
+        self.network.add_router_to_agent = mock.Mock()
+        self.cmd = network_agent.AddRouterToAgent(self.app, self.namespace)
+        self.network.get_agent = mock.Mock(return_value=self._agent)
+        self.network.find_router = mock.Mock(return_value=self._router)
+
+    def test_add_no_options(self):
+        arglist = []
+        verifylist = []
+
+        # Missing agent ID will cause command to bail
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
+    def test_add_router_required_options(self):
+        arglist = [
+            self._agent.id,
+            self._router.id,
+            '--l3',
+        ]
+        verifylist = [
+            ('l3', True),
+            ('agent_id', self._agent.id),
+            ('router', self._router.id),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.add_router_to_agent.assert_called_with(
+            self._agent, self._router)
+        self.assertIsNone(result)
+
+
 class TestDeleteNetworkAgent(TestNetworkAgent):
 
     network_agents = (
@@ -171,34 +211,16 @@ class TestListNetworkAgent(TestNetworkAgent):
     )
     data = []
     for agent in network_agents:
-        agent.agent_type = 'DHCP agent'
         data.append((
             agent.id,
             agent.agent_type,
             agent.host,
             agent.availability_zone,
-            agent.alive,
+            network_agent._format_alive(agent.alive),
             network_agent._format_admin_state(agent.admin_state_up),
             agent.binary,
         ))
 
-    network_agent_columns = (
-        'ID',
-        'Host',
-        'Admin State Up',
-        'Alive',
-    )
-
-    network_agent_data = []
-
-    for agent in network_agents:
-        network_agent_data.append((
-            agent.id,
-            agent.host,
-            network_agent._format_admin_state(agent.admin_state_up),
-            agent.alive,
-        ))
-
     def setUp(self):
         super(TestListNetworkAgent, self).setUp()
         self.network.agents = mock.Mock(
@@ -213,6 +235,14 @@ class TestListNetworkAgent(TestNetworkAgent):
         self.network.network_hosting_dhcp_agents = mock.Mock(
             return_value=self.network_agents)
 
+        self.network.get_agent = mock.Mock(return_value=_testagent)
+
+        self._testrouter = \
+            network_fakes.FakeRouter.create_one_router()
+        self.network.find_router = mock.Mock(return_value=self._testrouter)
+        self.network.routers_hosting_l3_agents = mock.Mock(
+            return_value=self.network_agents)
+
         # Get the command object to test
         self.cmd = network_agent.ListNetworkAgent(self.app, self.namespace)
 
@@ -239,7 +269,7 @@ class TestListNetworkAgent(TestNetworkAgent):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.network.agents.assert_called_once_with(**{
-            'agent_type': self.network_agents[0].agent_type,
+            'agent_type': 'DHCP agent',
         })
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
@@ -276,8 +306,53 @@ class TestListNetworkAgent(TestNetworkAgent):
 
         self.network.network_hosting_dhcp_agents.assert_called_once_with(
             *attrs)
-        self.assertEqual(self.network_agent_columns, columns)
-        self.assertEqual(list(self.network_agent_data), list(data))
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, list(data))
+
+    def test_network_agents_list_routers(self):
+        arglist = [
+            '--router', self._testrouter.id,
+        ]
+        verifylist = [
+            ('router', self._testrouter.id),
+            ('long', False)
+        ]
+
+        attrs = {self._testrouter, }
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.routers_hosting_l3_agents.assert_called_once_with(
+            *attrs)
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, list(data))
+
+    def test_network_agents_list_routers_with_long_option(self):
+        arglist = [
+            '--router', self._testrouter.id,
+            '--long',
+        ]
+        verifylist = [
+            ('router', self._testrouter.id),
+            ('long', True)
+        ]
+
+        attrs = {self._testrouter, }
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.routers_hosting_l3_agents.assert_called_once_with(
+            *attrs)
+
+        # Add a column 'HA State' and corresponding data.
+        router_agent_columns = self.columns + ('HA State',)
+        router_agent_data = [d + ('',) for d in self.data]
+
+        self.assertEqual(router_agent_columns, columns)
+        self.assertEqual(router_agent_data, list(data))
 
 
 class TestRemoveNetworkFromAgent(TestNetworkAgent):
@@ -303,6 +378,16 @@ class TestRemoveNetworkFromAgent(TestNetworkAgent):
         self.assertRaises(tests_utils.ParserException, self.check_parser,
                           self.cmd, arglist, verifylist)
 
+    def test_network_agents_list_routers_no_arg(self):
+        arglist = [
+            '--routers',
+        ]
+        verifylist = []
+
+        # Missing required args should bail here
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
     def test_network_from_dhcp_agent(self):
         arglist = [
             '--dhcp',
@@ -322,6 +407,46 @@ class TestRemoveNetworkFromAgent(TestNetworkAgent):
             self.agent, self.net)
 
 
+class TestRemoveRouterAgent(TestNetworkAgent):
+    _router = network_fakes.FakeRouter.create_one_router()
+    _agent = network_fakes.FakeNetworkAgent.create_one_network_agent()
+
+    def setUp(self):
+        super(TestRemoveRouterAgent, self).setUp()
+        self.network.remove_router_from_agent = mock.Mock()
+        self.cmd = network_agent.RemoveRouterFromAgent(self.app,
+                                                       self.namespace)
+        self.network.get_agent = mock.Mock(return_value=self._agent)
+        self.network.find_router = mock.Mock(return_value=self._router)
+
+    def test_remove_no_options(self):
+        arglist = []
+        verifylist = []
+
+        # Missing agent ID will cause command to bail
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
+    def test_remove_router_required_options(self):
+        arglist = [
+            '--l3',
+            self._agent.id,
+            self._router.id,
+        ]
+        verifylist = [
+            ('l3', True),
+            ('agent_id', self._agent.id),
+            ('router', self._router.id),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.remove_router_from_agent.assert_called_with(
+            self._agent, self._router)
+        self.assertIsNone(result)
+
+
 class TestSetNetworkAgent(TestNetworkAgent):
 
     _network_agent = (
@@ -415,9 +540,9 @@ class TestShowNetworkAgent(TestNetworkAgent):
         'id',
     )
     data = (
-        network_agent._format_admin_state(_network_agent.admin_state_up),
+        network_agent._format_admin_state(_network_agent.is_admin_state_up),
         _network_agent.agent_type,
-        _network_agent.alive,
+        network_agent._format_alive(_network_agent.is_alive),
         _network_agent.availability_zone,
         _network_agent.binary,
         utils.format_dict(_network_agent.configurations),
diff --git a/openstackclient/tests/unit/network/v2/test_router.py b/openstackclient/tests/unit/network/v2/test_router.py
index 02e0be94..c153fe4a 100644
--- a/openstackclient/tests/unit/network/v2/test_router.py
+++ b/openstackclient/tests/unit/network/v2/test_router.py
@@ -381,6 +381,21 @@ class TestListRouter(TestRouter):
             r.ha,
             r.tenant_id,
         ))
+
+    router_agent_data = []
+    for r in routers:
+        router_agent_data.append((
+            r.id,
+            r.name,
+            r.external_gateway_info,
+        ))
+
+    agents_columns = (
+        'ID',
+        'Name',
+        'External Gateway Info',
+    )
+
     data_long = []
     for i in range(0, len(routers)):
         r = routers[i]
@@ -407,8 +422,15 @@ class TestListRouter(TestRouter):
         # Get the command object to test
         self.cmd = router.ListRouter(self.app, self.namespace)
 
+        self.network.agent_hosted_routers = mock.Mock(
+            return_value=self.routers)
         self.network.routers = mock.Mock(return_value=self.routers)
         self.network.find_extension = mock.Mock(return_value=self._extensions)
+        self.network.find_router = mock.Mock(return_value=self.routers[0])
+        self._testagent = \
+            network_fakes.FakeNetworkAgent.create_one_network_agent()
+        self.network.get_agent = mock.Mock(return_value=self._testagent)
+        self.network.get_router = mock.Mock(return_value=self.routers[0])
 
     def test_router_list_no_options(self):
         arglist = []
@@ -556,6 +578,34 @@ class TestListRouter(TestRouter):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
 
+    def test_router_list_agents_no_args(self):
+        arglist = [
+            '--agents',
+        ]
+        verifylist = []
+
+        # Missing required router ID should bail here
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
+    def test_router_list_agents(self):
+        arglist = [
+            '--agent', self._testagent.id,
+        ]
+        verifylist = [
+            ('agent', self._testagent.id),
+        ]
+
+        attrs = {self._testagent.id, }
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.agent_hosted_routers(
+            *attrs)
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, list(data))
+
 
 class TestRemovePortFromRouter(TestRouter):
     '''Remove port from a Router '''