From 9ffe53bac05b6bd69b0c44c814b9e407982f04ee Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Mon, 15 Dec 2014 12:48:47 -0800 Subject: [PATCH] Fix NetworkInUse when deleting RS Cloud::Network Adds retry logic to the deletion of Rackspace::Cloud::Network so that it doesn't throw NetworkInUse exceptions. Closes-Bug: 1402827 Change-Id: I5f1dc6d1c1cfc7d0b795fe43fdb677969372bd0b --- .../rackspace/resources/cloudnetworks.py | 49 ++++++++++++++----- .../rackspace/tests/test_cloudnetworks.py | 19 ++++++- 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/contrib/rackspace/rackspace/resources/cloudnetworks.py b/contrib/rackspace/rackspace/resources/cloudnetworks.py index 4769fcc05..cc4c0cac8 100644 --- a/contrib/rackspace/rackspace/resources/cloudnetworks.py +++ b/contrib/rackspace/rackspace/resources/cloudnetworks.py @@ -23,6 +23,7 @@ from heat.engine import resource from heat.openstack.common import log as logging try: + from pyrax.exceptions import NetworkInUse # noqa from pyrax.exceptions import NotFound # noqa PYRAX_INSTALLED = True except ImportError: @@ -31,13 +32,15 @@ except ImportError: class NotFound(Exception): """Dummy pyrax exception - only used for testing.""" + class NetworkInUse(Exception): + """Dummy pyrax exception - only used for testing.""" + LOG = logging.getLogger(__name__) class CloudNetwork(resource.Resource): - """ - A resource for creating Rackspace Cloud Networks. + """A resource for creating Rackspace Cloud Networks. See http://www.rackspace.com/cloud/networks/ for service documentation. @@ -106,20 +109,40 @@ class CloudNetwork(resource.Resource): self.cloud_networks().get(self.resource_id) def handle_delete(self): - net = self.network() - if net: - net.delete() - return net + '''Delete cloud network. - def check_delete_complete(self, network): - if network: + Cloud Network doesn't have a status attribute, and there is a non-zero + window between the deletion of a server and the acknowledgement from + the cloud network that it's no longer in use, so it needs some way to + keep track of when the delete call was successfully issued. + ''' + network_info = { + 'delete_issued': False, + 'network': self.network(), + } + return network_info + + def check_delete_complete(self, network_info): + network = network_info['network'] + + if not network: + return True + + if not network_info['delete_issued']: try: - network.get() - except NotFound: - return True + network.delete() + except NetworkInUse: + LOG.warn("Network '%s' still in use." % network.id) else: - return False - return True + network_info['delete_issued'] = True + return False + + try: + network.get() + except NotFound: + return True + + return False def validate(self): super(CloudNetwork, self).validate() diff --git a/contrib/rackspace/rackspace/tests/test_cloudnetworks.py b/contrib/rackspace/rackspace/tests/test_cloudnetworks.py index 6dbf960a8..ada78f67b 100644 --- a/contrib/rackspace/rackspace/tests/test_cloudnetworks.py +++ b/contrib/rackspace/rackspace/tests/test_cloudnetworks.py @@ -26,7 +26,7 @@ from heat.tests import utils from ..resources import cloudnetworks # noqa try: - from pyrax.exceptions import NotFound + from pyrax.exceptions import NotFound # noqa except ImportError: from ..resources.cloudnetworks import NotFound # noqa @@ -144,10 +144,25 @@ class CloudNetworkTest(common.HeatTestCase): exc = self.assertRaises(NotFound, self.fake_cnw.get, res_id) self.assertIn(res_id, six.text_type(exc)) + def test_delete_in_use(self, mock_client): + self._setup_stack(mock_client) + res = self.stack['cnw'] + fake_network = res.network() + fake_network.delete = mock.Mock() + fake_network.delete.side_effect = [cloudnetworks.NetworkInUse(), True] + fake_network.get = mock.Mock(side_effect=cloudnetworks.NotFound()) + + scheduler.TaskRunner(res.delete)() + self.assertEqual((res.DELETE, res.COMPLETE), res.state) + def test_delete_not_complete(self, mock_client): self._setup_stack(mock_client) res = self.stack['cnw'] - self.assertFalse(res.check_delete_complete(res.network())) + fake_network = res.network() + fake_network.get = mock.Mock() + + task = res.handle_delete() + self.assertFalse(res.check_delete_complete(task)) def test_delete_not_found(self, mock_client): self._setup_stack(mock_client)