Error handling of "router delete" command

Huanxuan Ao · Huanxuan Ao · commit 8b6626e9b60a · 2016-06-20T18:05:20.000+08:00
"Router delete" command supports multi deletion but no error
handling. This patch add the error handling follow the rule
in doc/source/command-error.rst

Change-Id: I3376d957b4dc28d8282599dc909ecc5ed2b5f46a
diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py
@@ -19,6 +19,7 @@
 
 from osc_lib.cli import parseractions
 from osc_lib.command import command
+from osc_lib import exceptions
 from osc_lib import utils
 
 from openstackclient.i18n import _
@@ -222,9 +223,23 @@ def get_parser(self, prog_name):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
+        result = 0
+
         for router in parsed_args.router:
-            obj = client.find_router(router)
-            client.delete_router(obj)
+            try:
+                obj = client.find_router(router, ignore_missing=False)
+                client.delete_router(obj)
+            except Exception as e:
+                result += 1
+                LOG.error(_("Failed to delete router with "
+                          "name or ID '%(router)s': %(e)s")
+                          % {'router': router, 'e': e})
+
+        if result > 0:
+            total = len(parsed_args.router)
+            msg = (_("%(result)s of %(total)s routers failed "
+                   "to delete.") % {'result': result, 'total': total})
+            raise exceptions.CommandError(msg)
 
 
 class ListRouter(command.Lister):
diff --git a/openstackclient/tests/network/v2/test_router.py b/openstackclient/tests/network/v2/test_router.py
@@ -12,7 +12,9 @@
 #
 
 import mock
+from mock import call
 
+from osc_lib import exceptions
 from osc_lib import utils as osc_utils
 
 from openstackclient.network.v2 import router
@@ -202,32 +204,82 @@ def test_create_with_AZ_hints(self):
 
 class TestDeleteRouter(TestRouter):
 
-    # The router to delete.
-    _router = network_fakes.FakeRouter.create_one_router()
+    # The routers to delete.
+    _routers = network_fakes.FakeRouter.create_routers(count=2)
 
     def setUp(self):
         super(TestDeleteRouter, self).setUp()
 
         self.network.delete_router = mock.Mock(return_value=None)
 
-        self.network.find_router = mock.Mock(return_value=self._router)
+        self.network.find_router = (
+            network_fakes.FakeRouter.get_routers(self._routers))
 
         # Get the command object to test
         self.cmd = router.DeleteRouter(self.app, self.namespace)
 
-    def test_delete(self):
+    def test_router_delete(self):
         arglist = [
-            self._router.name,
+            self._routers[0].name,
         ]
         verifylist = [
-            ('router', [self._router.name]),
+            ('router', [self._routers[0].name]),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.network.delete_router.assert_called_once_with(self._routers[0])
+        self.assertIsNone(result)
+
+    def test_multi_routers_delete(self):
+        arglist = []
+        verifylist = []
+
+        for r in self._routers:
+            arglist.append(r.name)
+        verifylist = [
+            ('router', arglist),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.network.delete_router.assert_called_once_with(self._router)
+
+        calls = []
+        for r in self._routers:
+            calls.append(call(r))
+        self.network.delete_router.assert_has_calls(calls)
         self.assertIsNone(result)
 
+    def test_multi_routers_delete_with_exception(self):
+        arglist = [
+            self._routers[0].name,
+            'unexist_router',
+        ]
+        verifylist = [
+            ('router',
+             [self._routers[0].name, 'unexist_router']),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        find_mock_result = [self._routers[0], exceptions.CommandError]
+        self.network.find_router = (
+            mock.MagicMock(side_effect=find_mock_result)
+        )
+
+        try:
+            self.cmd.take_action(parsed_args)
+            self.fail('CommandError should be raised.')
+        except exceptions.CommandError as e:
+            self.assertEqual('1 of 2 routers failed to delete.', str(e))
+
+        self.network.find_router.assert_any_call(
+            self._routers[0].name, ignore_missing=False)
+        self.network.find_router.assert_any_call(
+            'unexist_router', ignore_missing=False)
+        self.network.delete_router.assert_called_once_with(
+            self._routers[0]
+        )
+
 
 class TestListRouter(TestRouter):
 
