diff --git a/doc/source/command-errors.rst b/doc/source/command-errors.rst
index 35d7f9d072..a24dc317bb 100644
--- a/doc/source/command-errors.rst
+++ b/doc/source/command-errors.rst
@@ -99,8 +99,8 @@ to be handled by having the proper options in a set command available to allow
recovery in the case where the primary resource has been created but the
subsequent calls did not complete.
-Example
-~~~~~~~
+Example 1
+~~~~~~~~~
This example is taken from the ``volume snapshot set`` command where ``--property``
arguments are set using the volume manager's ``set_metadata()`` method,
@@ -161,3 +161,41 @@ remaining arguments are set using the ``update()`` method.
# without aborting prematurely
if result > 0:
raise SomeNonFatalException
+
+Example 2
+~~~~~~~~~
+
+This example is taken from the ``network delete`` command which takes multiple
+networks to delete. All networks will be delete in a loop, which makes multiple
+``delete_network()`` calls.
+
+.. code-block:: python
+
+ class DeleteNetwork(common.NetworkAndComputeCommand):
+ """Delete network(s)"""
+
+ def update_parser_common(self, parser):
+ parser.add_argument(
+ 'network',
+ metavar="<network>",
+ nargs="+",
+ help=("Network(s) to delete (name or ID)")
+ )
+ return parser
+
+ def take_action(self, client, parsed_args):
+ ret = 0
+
+ for network in parsed_args.network:
+ try:
+ obj = client.find_network(network, ignore_missing=False)
+ client.delete_network(obj)
+ except Exception:
+ self.app.log.error("Failed to delete network with name "
+ "or ID %s." % network)
+ ret += 1
+
+ if ret > 0:
+ total = len(parsed_args.network)
+ msg = "Failed to delete %s of %s networks." % (ret, total)
+ raise exceptions.CommandError(msg)
diff --git a/openstackclient/network/common.py b/openstackclient/network/common.py
index 1e2c4cce99..a3047d84ba 100644
--- a/openstackclient/network/common.py
+++ b/openstackclient/network/common.py
@@ -15,6 +15,7 @@ import abc
import six
from openstackclient.common import command
+from openstackclient.common import exceptions
@six.add_metaclass(abc.ABCMeta)
@@ -68,6 +69,42 @@ class NetworkAndComputeCommand(command.Command):
pass
+@six.add_metaclass(abc.ABCMeta)
+class NetworkAndComputeDelete(NetworkAndComputeCommand):
+ """Network and Compute Delete
+
+ Delete class for commands that support implementation via
+ the network or compute endpoint. Such commands have different
+ implementations for take_action() and may even have different
+ arguments. This class supports bulk deletion, and error handling
+ following the rules in doc/source/command-errors.rst.
+ """
+
+ def take_action(self, parsed_args):
+ ret = 0
+ resources = getattr(parsed_args, self.resource, [])
+
+ for r in resources:
+ self.r = r
+ try:
+ if self.app.client_manager.is_network_endpoint_enabled():
+ self.take_action_network(self.app.client_manager.network,
+ parsed_args)
+ else:
+ self.take_action_compute(self.app.client_manager.compute,
+ parsed_args)
+ except Exception as e:
+ self.app.log.error("Failed to delete %s with name or ID "
+ "'%s': %s" % (self.resource, r, e))
+ ret += 1
+
+ if ret:
+ total = len(resources)
+ msg = "%s of %s %ss failed to delete." % (ret, total,
+ self.resource)
+ raise exceptions.CommandError(msg)
+
+
@six.add_metaclass(abc.ABCMeta)
class NetworkAndComputeLister(command.Lister):
"""Network and Compute Lister
diff --git a/openstackclient/network/v2/network.py b/openstackclient/network/v2/network.py
index bdde91735d..bf01e2ec71 100644
--- a/openstackclient/network/v2/network.py
+++ b/openstackclient/network/v2/network.py
@@ -248,9 +248,13 @@ class CreateNetwork(common.NetworkAndComputeShowOne):
return (columns, data)
-class DeleteNetwork(common.NetworkAndComputeCommand):
+class DeleteNetwork(common.NetworkAndComputeDelete):
"""Delete network(s)"""
+ # Used by base class to find resources in parsed_args.
+ resource = 'network'
+ r = None
+
def update_parser_common(self, parser):
parser.add_argument(
'network',
@@ -258,20 +262,16 @@ class DeleteNetwork(common.NetworkAndComputeCommand):
nargs="+",
help=_("Network(s) to delete (name or ID)")
)
+
return parser
def take_action_network(self, client, parsed_args):
- for network in parsed_args.network:
- obj = client.find_network(network)
- client.delete_network(obj)
+ obj = client.find_network(self.r, ignore_missing=False)
+ client.delete_network(obj)
def take_action_compute(self, client, parsed_args):
- for network in parsed_args.network:
- network = utils.find_resource(
- client.networks,
- network,
- )
- client.networks.delete(network.id)
+ network = utils.find_resource(client.networks, self.r)
+ client.networks.delete(network.id)
class ListNetwork(common.NetworkAndComputeLister):
diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py
index b3f3fb4917..70fc386f1b 100644
--- a/openstackclient/tests/compute/v2/fakes.py
+++ b/openstackclient/tests/compute/v2/fakes.py
@@ -859,6 +859,25 @@ class FakeNetwork(object):
return networks
+ @staticmethod
+ def get_networks(networks=None, count=2):
+ """Get an iterable MagicMock object with a list of faked networks.
+
+ If networks list is provided, then initialize the Mock object with the
+ list. Otherwise create one.
+
+ :param List networks:
+ A list of FakeResource objects faking networks
+ :param int count:
+ The number of networks to fake
+ :return:
+ An iterable Mock object with side_effect set to a list of faked
+ networks
+ """
+ if networks is None:
+ networks = FakeNetwork.create_networks(count=count)
+ return mock.Mock(side_effect=networks)
+
class FakeHost(object):
"""Fake one host."""
diff --git a/openstackclient/tests/network/v2/test_network.py b/openstackclient/tests/network/v2/test_network.py
index c6736c56f3..ba810f1649 100644
--- a/openstackclient/tests/network/v2/test_network.py
+++ b/openstackclient/tests/network/v2/test_network.py
@@ -14,6 +14,7 @@
import copy
import mock
+from mock import call
from openstackclient.common import exceptions
from openstackclient.common import utils
from openstackclient.network.v2 import network
@@ -323,33 +324,88 @@ class TestCreateNetworkIdentityV2(TestNetwork):
class TestDeleteNetwork(TestNetwork):
- # The network to delete.
- _network = network_fakes.FakeNetwork.create_one_network()
-
def setUp(self):
super(TestDeleteNetwork, self).setUp()
+ # The networks to delete
+ self._networks = network_fakes.FakeNetwork.create_networks(count=3)
+
self.network.delete_network = mock.Mock(return_value=None)
- self.network.find_network = mock.Mock(return_value=self._network)
+ self.network.find_network = network_fakes.FakeNetwork.get_networks(
+ networks=self._networks)
# Get the command object to test
self.cmd = network.DeleteNetwork(self.app, self.namespace)
- def test_delete(self):
+ def test_delete_one_network(self):
arglist = [
- self._network.name,
+ self._networks[0].name,
]
verifylist = [
- ('network', [self._network.name]),
+ ('network', [self._networks[0].name]),
]
-
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
result = self.cmd.take_action(parsed_args)
- self.network.delete_network.assert_called_once_with(self._network)
+ self.network.delete_network.assert_called_once_with(self._networks[0])
self.assertIsNone(result)
+ def test_delete_multiple_networks(self):
+ arglist = []
+ for n in self._networks:
+ arglist.append(n.id)
+ verifylist = [
+ ('network', arglist),
+ ]
+ parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+ result = self.cmd.take_action(parsed_args)
+
+ calls = []
+ for n in self._networks:
+ calls.append(call(n))
+ self.network.delete_network.assert_has_calls(calls)
+ self.assertIsNone(result)
+
+ def test_delete_multiple_networks_exception(self):
+ arglist = [
+ self._networks[0].id,
+ 'xxxx-yyyy-zzzz',
+ self._networks[1].id,
+ ]
+ verifylist = [
+ ('network', arglist),
+ ]
+ parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+ # Fake exception in find_network()
+ ret_find = [
+ self._networks[0],
+ exceptions.NotFound('404'),
+ self._networks[1],
+ ]
+ self.network.find_network = mock.Mock(side_effect=ret_find)
+
+ # Fake exception in delete_network()
+ ret_delete = [
+ None,
+ exceptions.NotFound('404'),
+ ]
+ self.network.delete_network = mock.Mock(side_effect=ret_delete)
+
+ self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+ parsed_args)
+
+ # The second call of find_network() should fail. So delete_network()
+ # was only called twice.
+ calls = [
+ call(self._networks[0]),
+ call(self._networks[1]),
+ ]
+ self.network.delete_network.assert_has_calls(calls)
+
class TestListNetwork(TestNetwork):
@@ -752,36 +808,97 @@ class TestCreateNetworkCompute(TestNetworkCompute):
class TestDeleteNetworkCompute(TestNetworkCompute):
- # The network to delete.
- _network = compute_fakes.FakeNetwork.create_one_network()
-
def setUp(self):
super(TestDeleteNetworkCompute, self).setUp()
self.app.client_manager.network_endpoint_enabled = False
+ # The networks to delete
+ self._networks = compute_fakes.FakeNetwork.create_networks(count=3)
+
self.compute.networks.delete.return_value = None
# Return value of utils.find_resource()
- self.compute.networks.get.return_value = self._network
+ self.compute.networks.get = \
+ compute_fakes.FakeNetwork.get_networks(networks=self._networks)
# Get the command object to test
self.cmd = network.DeleteNetwork(self.app, None)
- def test_network_delete(self):
+ def test_delete_one_network(self):
arglist = [
- self._network.label,
+ self._networks[0].label,
]
verifylist = [
- ('network', [self._network.label]),
+ ('network', [self._networks[0].label]),
]
-
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
result = self.cmd.take_action(parsed_args)
- self.compute.networks.delete.assert_called_once_with(self._network.id)
+ self.compute.networks.delete.assert_called_once_with(
+ self._networks[0].id)
self.assertIsNone(result)
+ def test_delete_multiple_networks(self):
+ arglist = []
+ for n in self._networks:
+ arglist.append(n.label)
+ verifylist = [
+ ('network', arglist),
+ ]
+ parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+ result = self.cmd.take_action(parsed_args)
+
+ calls = []
+ for n in self._networks:
+ calls.append(call(n.id))
+ self.compute.networks.delete.assert_has_calls(calls)
+ self.assertIsNone(result)
+
+ def test_delete_multiple_networks_exception(self):
+ arglist = [
+ self._networks[0].id,
+ 'xxxx-yyyy-zzzz',
+ self._networks[1].id,
+ ]
+ verifylist = [
+ ('network', arglist),
+ ]
+ parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+ # Fake exception in utils.find_resource()
+ # In compute v2, we use utils.find_resource() to find a network.
+ # It calls get() several times, but find() only one time. So we
+ # choose to fake get() always raise exception, then pass through.
+ # And fake find() to find the real network or not.
+ self.compute.networks.get.side_effect = Exception()
+ ret_find = [
+ self._networks[0],
+ Exception(),
+ self._networks[1],
+ ]
+ self.compute.networks.find.side_effect = ret_find
+
+ # Fake exception in delete()
+ ret_delete = [
+ None,
+ Exception(),
+ ]
+ self.compute.networks.delete = mock.Mock(side_effect=ret_delete)
+
+ self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+ parsed_args)
+
+ # The second call of utils.find_resource() should fail. So delete()
+ # was only called twice.
+ calls = [
+ call(self._networks[0].id),
+ call(self._networks[1].id),
+ ]
+ self.compute.networks.delete.assert_has_calls(calls)
+
class TestListNetworkCompute(TestNetworkCompute):
diff --git a/releasenotes/notes/bug-1556719-d2dcf61acf87e856.yaml b/releasenotes/notes/bug-1556719-d2dcf61acf87e856.yaml
new file mode 100644
index 0000000000..7c8e5c0628
--- /dev/null
+++ b/releasenotes/notes/bug-1556719-d2dcf61acf87e856.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+ - Command ``network delete`` will delete as many networks as possible, log
+ and report failures in the end.
+ [Bug `1556719 <https://bugs.launchpad.net/python-openstackclient/+bug/1556719>`_]
+ [Bug `1537856 <https://bugs.launchpad.net/python-openstackclient/+bug/1537856>`_]