Merge "Reset the client object after proxy timeouts"

nodepool/fakeprovider.py

@@ -16,6 +16,7 @@
 
 import StringIO
 import novaclient
+import requests.exceptions
 import threading
 import time
 import uuid
@@ -118,6 +119,12 @@ class FakeHTTPClient(object):
             return None, dict(extensions=dict())
 
 
+class BadHTTPClient(object):
+    '''Always raises a ProxyError'''
+    def get(self, path):
+        raise requests.exceptions.ProxyError
+
+
 class FakeClient(object):
     def __init__(self, *args, **kwargs):
         self.flavors = FakeList([
@@ -130,6 +137,12 @@ class FakeClient(object):
         self.servers.api = self
 
 
+class BadClient(FakeClient):
+    def __init__(self):
+        super(BadClient, self).__init__()
+        self.client = BadHTTPClient()
+
+
 class FakeGlanceClient(object):
     def __init__(self, *args, **kwargs):
         self.id = 'fake-glance-id'
@@ -226,3 +239,4 @@ class FakeJenkins(object):
 
 
 FAKE_CLIENT = FakeClient()
+BAD_CLIENT = BadClient()

nodepool/provider_manager.py

@@ -27,6 +27,8 @@ import glanceclient
 import glanceclient.client
 import keystoneclient.v2_0.client as ksclient
 import time
+import requests.exceptions
+import sys
 
 from nodeutils import iterate_timeout
 from task_manager import Task, TaskManager, ManagerStoppedException
@@ -251,7 +253,7 @@ class ProviderManager(TaskManager):
         super(ProviderManager, self).__init__(None, provider.name,
                                               provider.rate)
         self.provider = provider
-        self._client = self._getClient()
+        self.resetClient()
         self._images = {}
         self._networks = {}
         self._cloud_metadata_read = False
@@ -297,6 +299,24 @@ class ProviderManager(TaskManager):
             kwargs['timeout'] = self.provider.api_timeout
         return novaclient.client.Client(*args, **kwargs)
 
+    def runTask(self, task):
+        try:
+            task.run(self._client)
+        except requests.exceptions.ProxyError:
+            # Try to get a new client object if we get a ProxyError
+            self.log.exception('Resetting client due to ProxyError')
+            self.resetClient()
+            try:
+                task.run(self._client)
+            except requests.exceptions.ProxyError as e:
+                # If we get a second ProxyError, then make sure it gets raised
+                # the same way all other Exceptions from the Task object do.
+                # This will move the Exception to the main thread.
+                task.exception(e, sys.exc_info()[2])
+
+    def resetClient(self):
+        self._client = self._getClient()
+
     def _getFlavors(self):
         flavors = self.listFlavors()
         flavors.sort(lambda a, b: cmp(a['ram'], b['ram']))

nodepool/task_manager.py

@@ -22,6 +22,7 @@ from six.moves import queue as Queue
 import logging
 import time
 from statsd import statsd
+import requests.exceptions
 
 
 class ManagerStoppedException(Exception):
@@ -54,6 +55,8 @@ class Task(object):
     def run(self, client):
         try:
             self.done(self.main(client))
+        except requests.exceptions.ProxyError as e:
+            raise e
         except Exception as e:
             self.exception(e, sys.exc_info()[2])
 
@@ -90,7 +93,7 @@ class TaskManager(threading.Thread):
             self.log.debug("Manager %s running task %s (queue: %s)" %
                            (self.name, task, self.queue.qsize()))
             start = time.time()
-            task.run(self._client)
+            self.runTask(task)
             last_ts = time.time()
             dt = last_ts - start
             self.log.debug("Manager %s ran task %s in %ss" %
@@ -110,3 +113,6 @@ class TaskManager(threading.Thread):
                 "Manager %s is no longer running" % self.name)
         self.queue.put(task)
         return task.wait()
+
+    def runTask(self, task):
+        task.run(self._client)

nodepool/tests/test_nodepool.py

@@ -17,8 +17,12 @@ import fixtures
 
 from nodepool import tests
 from nodepool import nodedb
+import nodepool.fakeprovider
 import nodepool.nodepool
 
+import requests.exceptions
+from testtools import ExpectedException
+
 
 class TestNodepool(tests.DBTestCase):
     def test_db(self):
@@ -279,3 +283,32 @@ class TestNodepool(tests.DBTestCase):
             # Make sure our old node is in delete state
             self.assertEqual(len(deleted_nodes), 1)
             self.assertEqual(node_id, deleted_nodes[0].id)
+
+    def test_proxy_timeout(self):
+        """Test that we re-run a task after a ProxyError"""
+        configfile = self.setup_config('node.yaml')
+        pool = self.useNodepool(configfile, watermark_sleep=1)
+        pool.start()
+        self.waitForNodes(pool)
+
+        provider = pool.config.providers['fake-provider']
+        manager = pool.getProviderManager(provider)
+
+        # In order to test recovering from a ProxyError from the client
+        # we are going manually set the client object to be a bad client that
+        # always raises a ProxyError. If our client reset works correctly
+        # then we will create a new client object, which in this case would
+        # be a new fake client in place of the bad client.
+        manager._client = nodepool.fakeprovider.BAD_CLIENT
+
+        # The only implemented function for the fake and bad clients
+        # If we don't raise an uncaught exception, we pass
+        manager.listExtensions()
+
+        # Now let's do it again, but let's prevent the client object from being
+        # replaced and then assert that we raised the exception that we expect.
+        manager._client = nodepool.fakeprovider.BAD_CLIENT
+        manager._getClient = lambda: nodepool.fakeprovider.BAD_CLIENT
+
+        with ExpectedException(requests.exceptions.ProxyError):
+            manager.listExtensions()