From eb421f6dabead63318139e2db5d0d11498c40a68 Mon Sep 17 00:00:00 2001
From: Huanxuan Ao <huanxuan.ao@easystack.cn>
Date: Fri, 3 Jun 2016 17:29:41 +0800
Subject: [PATCH] Support error handling for "port delete" command

"Port delete" command supported deleting multi ports
before but didn't support error handing, This patch
add the error handling following the rules in
doc/source/command-errors.rst.

Change-Id: I4ea69f2279763626d6a27cad1ca0ee99822d016d
Partially-Implements: blueprint multi-argument-network
---
 openstackclient/network/v2/port.py            | 17 ++++-
 openstackclient/tests/network/v2/test_port.py | 68 +++++++++++++++++--
 2 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py
index ca02281fd3..7ef3964bf9 100644
--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@@ -284,10 +284,23 @@ class DeletePort(command.Command):
 
     def take_action(self, parsed_args):
         client = self.app.client_manager.network
+        result = 0
 
         for port in parsed_args.port:
-            res = client.find_port(port)
-            client.delete_port(res)
+            try:
+                obj = client.find_port(port, ignore_missing=False)
+                client.delete_port(obj)
+            except Exception as e:
+                result += 1
+                self.app.log.error(_("Failed to delete port with "
+                                   "name or ID '%(port)s': %(e)s")
+                                   % {'port': port, 'e': e})
+
+        if result > 0:
+            total = len(parsed_args.port)
+            msg = (_("%(result)s of %(total)s ports failed "
+                   "to delete.") % {'result': result, 'total': total})
+            raise exceptions.CommandError(msg)
 
 
 class ListPort(command.Lister):
diff --git a/openstackclient/tests/network/v2/test_port.py b/openstackclient/tests/network/v2/test_port.py
index f2aa26cf88..bb23cf631c 100644
--- a/openstackclient/tests/network/v2/test_port.py
+++ b/openstackclient/tests/network/v2/test_port.py
@@ -13,6 +13,8 @@
 
 import mock
 
+from mock import call
+from openstackclient.common import exceptions
 from openstackclient.common import utils
 from openstackclient.network.v2 import port
 from openstackclient.tests.network.v2 import fakes as network_fakes
@@ -174,30 +176,82 @@ class TestCreatePort(TestPort):
 
 class TestDeletePort(TestPort):
 
-    # The port to delete.
-    _port = network_fakes.FakePort.create_one_port()
+    # Ports to delete.
+    _ports = network_fakes.FakePort.create_ports(count=2)
 
     def setUp(self):
         super(TestDeletePort, self).setUp()
 
         self.network.delete_port = mock.Mock(return_value=None)
-        self.network.find_port = mock.Mock(return_value=self._port)
+        self.network.find_port = network_fakes.FakePort.get_ports(
+            ports=self._ports)
         # Get the command object to test
         self.cmd = port.DeletePort(self.app, self.namespace)
 
-    def test_delete(self):
+    def test_port_delete(self):
         arglist = [
-            self._port.name,
+            self._ports[0].name,
         ]
         verifylist = [
-            ('port', [self._port.name]),
+            ('port', [self._ports[0].name]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.network.delete_port.assert_called_once_with(self._port)
+        self.network.find_port.assert_called_once_with(
+            self._ports[0].name, ignore_missing=False)
+        self.network.delete_port.assert_called_once_with(self._ports[0])
         self.assertIsNone(result)
 
+    def test_multi_ports_delete(self):
+        arglist = []
+        verifylist = []
+
+        for p in self._ports:
+            arglist.append(p.name)
+        verifylist = [
+            ('port', arglist),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        calls = []
+        for p in self._ports:
+            calls.append(call(p))
+        self.network.delete_port.assert_has_calls(calls)
+        self.assertIsNone(result)
+
+    def test_multi_ports_delete_with_exception(self):
+        arglist = [
+            self._ports[0].name,
+            'unexist_port',
+        ]
+        verifylist = [
+            ('port',
+             [self._ports[0].name, 'unexist_port']),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        find_mock_result = [self._ports[0], exceptions.CommandError]
+        self.network.find_port = (
+            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 ports failed to delete.', str(e))
+
+        self.network.find_port.assert_any_call(
+            self._ports[0].name, ignore_missing=False)
+        self.network.find_port.assert_any_call(
+            'unexist_port', ignore_missing=False)
+        self.network.delete_port.assert_called_once_with(
+            self._ports[0]
+        )
+
 
 class TestListPort(TestPort):