From 1dc1d25b8e1510e910f62a7ebb981af0298e336c Mon Sep 17 00:00:00 2001
From: Clint Byrum <clint@fewbar.com>
Date: Wed, 13 May 2015 14:11:59 -0700
Subject: [PATCH] Handle novaclient exception in delete_server wait

When a wait is requested, we poll the nova server with the ID of the
deleted server until it disappears. We might get an error on that poll,
and we should wrap that exception to avoid leaking client exceptions.

Change-Id: I6474bf37835728f70210ad29a3757a4488dfe037
---
 shade/__init__.py                      |  5 ++++
 shade/tests/unit/test_delete_server.py | 38 ++++++++++++++++++++------
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/shade/__init__.py b/shade/__init__.py
index 663b06045..7a99debe6 100644
--- a/shade/__init__.py
+++ b/shade/__init__.py
@@ -1769,6 +1769,11 @@ class OpenStackCloud(object):
                     return
             except nova_exceptions.NotFound:
                 return
+            except Exception as e:
+                self.log.debug("nova get server failed when waiting for "
+                               "delete", exc_info=True)
+                raise OpenStackCloudException(
+                    "Error in deleting server: {0}".format(e))
 
     def get_container(self, name, skip_cache=False):
         if skip_cache or name not in self._container_cache:
diff --git a/shade/tests/unit/test_delete_server.py b/shade/tests/unit/test_delete_server.py
index 0b8c18e3b..0ea2b4902 100644
--- a/shade/tests/unit/test_delete_server.py
+++ b/shade/tests/unit/test_delete_server.py
@@ -29,6 +29,14 @@ from shade.tests.unit import base
 
 
 class TestDeleteServer(base.TestCase):
+    novaclient_exceptions = (nova_exc.BadRequest,
+                             nova_exc.Unauthorized,
+                             nova_exc.Forbidden,
+                             nova_exc.MethodNotAllowed,
+                             nova_exc.Conflict,
+                             nova_exc.OverLimit,
+                             nova_exc.RateLimit,
+                             nova_exc.HTTPNotImplemented)
 
     def setUp(self):
         super(TestDeleteServer, self).setUp()
@@ -89,14 +97,7 @@ class TestDeleteServer(base.TestCase):
         nova_mock.servers.list.return_value = [fakes.FakeServer('1212',
                                                                 'speedy',
                                                                 'ACTIVE')]
-        for fail in (nova_exc.BadRequest,
-                     nova_exc.Unauthorized,
-                     nova_exc.Forbidden,
-                     nova_exc.MethodNotAllowed,
-                     nova_exc.Conflict,
-                     nova_exc.OverLimit,
-                     nova_exc.RateLimit,
-                     nova_exc.HTTPNotImplemented):
+        for fail in self.novaclient_exceptions:
 
             def _raise_fail(server):
                 raise fail(code=fail.http_status)
@@ -108,3 +109,24 @@ class TestDeleteServer(base.TestCase):
             # Note that message is deprecated from Exception, but not in
             # the novaclient exceptions.
             self.assertIn(fail.message, str(exc))
+
+    @mock.patch('shade.OpenStackCloud.nova_client')
+    def test_delete_server_get_fails(self, nova_mock):
+        """
+        Test that delete_server wraps novaclient exceptions on wait fails
+        """
+        nova_mock.servers.list.return_value = [fakes.FakeServer('2000',
+                                                                'yosemite',
+                                                                'ACTIVE')]
+        for fail in self.novaclient_exceptions:
+
+            def _raise_fail(server):
+                raise fail(code=fail.http_status)
+
+            nova_mock.servers.get.side_effect = _raise_fail
+            exc = self.assertRaises(shade_exc.OpenStackCloudException,
+                                    self.cloud.delete_server, 'yosemite',
+                                    wait=True)
+            # Note that message is deprecated from Exception, but not in
+            # the novaclient exceptions.
+            self.assertIn(fail.message, str(exc))