Skip to content

Commit 56f9227

Browse files
author
Tang Chen
committed
Enhance exception handling for "network delete" command
This patch rework "network delete" command following the rules in doc/source/command-errors.rst. In "network delete" command, there are multiple REST API calls, and we should make as many of them as possible. And log error for each one, give a better error message. Also return a non-zero exit code. Change-Id: I39ae087dd7bd08d049d513abfa6c5cab2bd13b2b Partial-Bug: #1556719
1 parent be6027e commit 56f9227

6 files changed

Lines changed: 245 additions & 28 deletions

File tree

‎doc/source/command-errors.rst‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ to be handled by having the proper options in a set command available to allow
9999
recovery in the case where the primary resource has been created but the
100100
subsequent calls did not complete.
101101

102-
Example
103-
~~~~~~~
102+
Example 1
103+
~~~~~~~~~
104104

105105
This example is taken from the ``volume snapshot set`` command where ``--property``
106106
arguments are set using the volume manager's ``set_metadata()`` method,
@@ -161,3 +161,41 @@ remaining arguments are set using the ``update()`` method.
161161
# without aborting prematurely
162162
if result > 0:
163163
raise SomeNonFatalException
164+
165+
Example 2
166+
~~~~~~~~~
167+
168+
This example is taken from the ``network delete`` command which takes multiple
169+
networks to delete. All networks will be delete in a loop, which makes multiple
170+
``delete_network()`` calls.
171+
172+
.. code-block:: python
173+
174+
class DeleteNetwork(common.NetworkAndComputeCommand):
175+
"""Delete network(s)"""
176+
177+
def update_parser_common(self, parser):
178+
parser.add_argument(
179+
'network',
180+
metavar="<network>",
181+
nargs="+",
182+
help=("Network(s) to delete (name or ID)")
183+
)
184+
return parser
185+
186+
def take_action(self, client, parsed_args):
187+
ret = 0
188+
189+
for network in parsed_args.network:
190+
try:
191+
obj = client.find_network(network, ignore_missing=False)
192+
client.delete_network(obj)
193+
except Exception:
194+
self.app.log.error("Failed to delete network with name "
195+
"or ID %s." % network)
196+
ret += 1
197+
198+
if ret > 0:
199+
total = len(parsed_args.network)
200+
msg = "Failed to delete %s of %s networks." % (ret, total)
201+
raise exceptions.CommandError(msg)

‎openstackclient/network/common.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import six
1616

1717
from openstackclient.common import command
18+
from openstackclient.common import exceptions
1819

1920

2021
@six.add_metaclass(abc.ABCMeta)
@@ -68,6 +69,42 @@ def take_action_compute(self, client, parsed_args):
6869
pass
6970

7071

72+
@six.add_metaclass(abc.ABCMeta)
73+
class NetworkAndComputeDelete(NetworkAndComputeCommand):
74+
"""Network and Compute Delete
75+
76+
Delete class for commands that support implementation via
77+
the network or compute endpoint. Such commands have different
78+
implementations for take_action() and may even have different
79+
arguments. This class supports bulk deletion, and error handling
80+
following the rules in doc/source/command-errors.rst.
81+
"""
82+
83+
def take_action(self, parsed_args):
84+
ret = 0
85+
resources = getattr(parsed_args, self.resource, [])
86+
87+
for r in resources:
88+
self.r = r
89+
try:
90+
if self.app.client_manager.is_network_endpoint_enabled():
91+
self.take_action_network(self.app.client_manager.network,
92+
parsed_args)
93+
else:
94+
self.take_action_compute(self.app.client_manager.compute,
95+
parsed_args)
96+
except Exception as e:
97+
self.app.log.error("Failed to delete %s with name or ID "
98+
"'%s': %s" % (self.resource, r, e))
99+
ret += 1
100+
101+
if ret:
102+
total = len(resources)
103+
msg = "%s of %s %ss failed to delete." % (ret, total,
104+
self.resource)
105+
raise exceptions.CommandError(msg)
106+
107+
71108
@six.add_metaclass(abc.ABCMeta)
72109
class NetworkAndComputeLister(command.Lister):
73110
"""Network and Compute Lister

‎openstackclient/network/v2/network.py‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,30 +224,30 @@ def take_action_compute(self, client, parsed_args):
224224
return (columns, data)
225225

226226

227-
class DeleteNetwork(common.NetworkAndComputeCommand):
227+
class DeleteNetwork(common.NetworkAndComputeDelete):
228228
"""Delete network(s)"""
229229

230+
# Used by base class to find resources in parsed_args.
231+
resource = 'network'
232+
r = None
233+
230234
def update_parser_common(self, parser):
231235
parser.add_argument(
232236
'network',
233237
metavar="<network>",
234238
nargs="+",
235239
help=("Network(s) to delete (name or ID)")
236240
)
241+
237242
return parser
238243

239244
def take_action_network(self, client, parsed_args):
240-
for network in parsed_args.network:
241-
obj = client.find_network(network)
242-
client.delete_network(obj)
245+
obj = client.find_network(self.r, ignore_missing=False)
246+
client.delete_network(obj)
243247

244248
def take_action_compute(self, client, parsed_args):
245-
for network in parsed_args.network:
246-
network = utils.find_resource(
247-
client.networks,
248-
network,
249-
)
250-
client.networks.delete(network.id)
249+
network = utils.find_resource(client.networks, self.r)
250+
client.networks.delete(network.id)
251251

252252

253253
class ListNetwork(common.NetworkAndComputeLister):

‎openstackclient/tests/compute/v2/fakes.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,25 @@ def create_networks(attrs={}, methods={}, count=2):
910910

911911
return networks
912912

913+
@staticmethod
914+
def get_networks(networks=None, count=2):
915+
"""Get an iterable MagicMock object with a list of faked networks.
916+
917+
If networks list is provided, then initialize the Mock object with the
918+
list. Otherwise create one.
919+
920+
:param List networks:
921+
A list of FakeResource objects faking networks
922+
:param int count:
923+
The number of networks to fake
924+
:return:
925+
An iterable Mock object with side_effect set to a list of faked
926+
networks
927+
"""
928+
if networks is None:
929+
networks = FakeNetwork.create_networks(count=count)
930+
return mock.Mock(side_effect=networks)
931+
913932

914933
class FakeHost(object):
915934
"""Fake one host."""

‎openstackclient/tests/network/v2/test_network.py‎

Lines changed: 133 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import copy
1515
import mock
1616

17+
from mock import call
1718
from openstackclient.common import exceptions
1819
from openstackclient.common import utils
1920
from openstackclient.network.v2 import network
@@ -321,33 +322,88 @@ def test_create_with_domain_identityv2(self):
321322

322323
class TestDeleteNetwork(TestNetwork):
323324

324-
# The network to delete.
325-
_network = network_fakes.FakeNetwork.create_one_network()
326-
327325
def setUp(self):
328326
super(TestDeleteNetwork, self).setUp()
329327

328+
# The networks to delete
329+
self._networks = network_fakes.FakeNetwork.create_networks(count=3)
330+
330331
self.network.delete_network = mock.Mock(return_value=None)
331332

332-
self.network.find_network = mock.Mock(return_value=self._network)
333+
self.network.find_network = network_fakes.FakeNetwork.get_networks(
334+
networks=self._networks)
333335

334336
# Get the command object to test
335337
self.cmd = network.DeleteNetwork(self.app, self.namespace)
336338

337-
def test_delete(self):
339+
def test_delete_one_network(self):
338340
arglist = [
339-
self._network.name,
341+
self._networks[0].name,
340342
]
341343
verifylist = [
342-
('network', [self._network.name]),
344+
('network', [self._networks[0].name]),
343345
]
346+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
347+
348+
result = self.cmd.take_action(parsed_args)
344349

350+
self.network.delete_network.assert_called_once_with(self._networks[0])
351+
self.assertIsNone(result)
352+
353+
def test_delete_multiple_networks(self):
354+
arglist = []
355+
for n in self._networks:
356+
arglist.append(n.id)
357+
verifylist = [
358+
('network', arglist),
359+
]
345360
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
361+
346362
result = self.cmd.take_action(parsed_args)
347363

348-
self.network.delete_network.assert_called_once_with(self._network)
364+
calls = []
365+
for n in self._networks:
366+
calls.append(call(n))
367+
self.network.delete_network.assert_has_calls(calls)
349368
self.assertIsNone(result)
350369

370+
def test_delete_multiple_networks_exception(self):
371+
arglist = [
372+
self._networks[0].id,
373+
'xxxx-yyyy-zzzz',
374+
self._networks[1].id,
375+
]
376+
verifylist = [
377+
('network', arglist),
378+
]
379+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
380+
381+
# Fake exception in find_network()
382+
ret_find = [
383+
self._networks[0],
384+
exceptions.NotFound('404'),
385+
self._networks[1],
386+
]
387+
self.network.find_network = mock.Mock(side_effect=ret_find)
388+
389+
# Fake exception in delete_network()
390+
ret_delete = [
391+
None,
392+
exceptions.NotFound('404'),
393+
]
394+
self.network.delete_network = mock.Mock(side_effect=ret_delete)
395+
396+
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
397+
parsed_args)
398+
399+
# The second call of find_network() should fail. So delete_network()
400+
# was only called twice.
401+
calls = [
402+
call(self._networks[0]),
403+
call(self._networks[1]),
404+
]
405+
self.network.delete_network.assert_has_calls(calls)
406+
351407

352408
class TestListNetwork(TestNetwork):
353409

@@ -730,36 +786,97 @@ def test_create_default_options(self):
730786

731787
class TestDeleteNetworkCompute(TestNetworkCompute):
732788

733-
# The network to delete.
734-
_network = compute_fakes.FakeNetwork.create_one_network()
735-
736789
def setUp(self):
737790
super(TestDeleteNetworkCompute, self).setUp()
738791

739792
self.app.client_manager.network_endpoint_enabled = False
740793

794+
# The networks to delete
795+
self._networks = compute_fakes.FakeNetwork.create_networks(count=3)
796+
741797
self.compute.networks.delete.return_value = None
742798

743799
# Return value of utils.find_resource()
744-
self.compute.networks.get.return_value = self._network
800+
self.compute.networks.get = \
801+
compute_fakes.FakeNetwork.get_networks(networks=self._networks)
745802

746803
# Get the command object to test
747804
self.cmd = network.DeleteNetwork(self.app, None)
748805

749-
def test_network_delete(self):
806+
def test_delete_one_network(self):
750807
arglist = [
751-
self._network.label,
808+
self._networks[0].label,
752809
]
753810
verifylist = [
754-
('network', [self._network.label]),
811+
('network', [self._networks[0].label]),
755812
]
813+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
814+
815+
result = self.cmd.take_action(parsed_args)
756816

817+
self.compute.networks.delete.assert_called_once_with(
818+
self._networks[0].id)
819+
self.assertIsNone(result)
820+
821+
def test_delete_multiple_networks(self):
822+
arglist = []
823+
for n in self._networks:
824+
arglist.append(n.label)
825+
verifylist = [
826+
('network', arglist),
827+
]
757828
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
829+
758830
result = self.cmd.take_action(parsed_args)
759831

760-
self.compute.networks.delete.assert_called_once_with(self._network.id)
832+
calls = []
833+
for n in self._networks:
834+
calls.append(call(n.id))
835+
self.compute.networks.delete.assert_has_calls(calls)
761836
self.assertIsNone(result)
762837

838+
def test_delete_multiple_networks_exception(self):
839+
arglist = [
840+
self._networks[0].id,
841+
'xxxx-yyyy-zzzz',
842+
self._networks[1].id,
843+
]
844+
verifylist = [
845+
('network', arglist),
846+
]
847+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
848+
849+
# Fake exception in utils.find_resource()
850+
# In compute v2, we use utils.find_resource() to find a network.
851+
# It calls get() several times, but find() only one time. So we
852+
# choose to fake get() always raise exception, then pass through.
853+
# And fake find() to find the real network or not.
854+
self.compute.networks.get.side_effect = Exception()
855+
ret_find = [
856+
self._networks[0],
857+
Exception(),
858+
self._networks[1],
859+
]
860+
self.compute.networks.find.side_effect = ret_find
861+
862+
# Fake exception in delete()
863+
ret_delete = [
864+
None,
865+
Exception(),
866+
]
867+
self.compute.networks.delete = mock.Mock(side_effect=ret_delete)
868+
869+
self.assertRaises(exceptions.CommandError, self.cmd.take_action,
870+
parsed_args)
871+
872+
# The second call of utils.find_resource() should fail. So delete()
873+
# was only called twice.
874+
calls = [
875+
call(self._networks[0].id),
876+
call(self._networks[1].id),
877+
]
878+
self.compute.networks.delete.assert_has_calls(calls)
879+
763880

764881
class TestListNetworkCompute(TestNetworkCompute):
765882

‎releasenotes/notes/bug-1556719-d2dcf61acf87e856.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- Command ``network delete`` will delete as many networks as possible, log
4+
and report failures in the end.
5+
[Bug `1556719 <https://bugs.launchpad.net/python-openstackclient/+bug/1556719>`_]
6+
[Bug `1537856 <https://bugs.launchpad.net/python-openstackclient/+bug/1537856>`_]

0 commit comments

Comments
 (0)