Retry requests to the remote cache server 3 times before failing

This makes it less likely that a build running overnight will fail due
to network gremlins.

2 files changed, 62 insertions, 24 deletions

diff --git a/morphlib/remoteartifactcache.py b/morphlib/remoteartifactcache.py
index 2be4ba70..a4caa27f 100644
--- a/morphlib/remoteartifactcache.py
+++ b/morphlib/remoteartifactcache.py
@@ -34,6 +34,10 @@ class GetError(cliapp.AppException):
 class RemoteArtifactCache(object):
 
     def __init__(self, server_url):
+        adapter = requests.adapters.HTTPAdapter(max_retries=3)
+        self.requests = requests.Session()
+        self.requests.mount('http://', adapter)
+
         self.server_url = server_url
 
     def has(self, artifact):
@@ -43,7 +47,7 @@ class RemoteArtifactCache(object):
         url = self._request_url(filename)
         logging.debug('RemoteArtifactCache._has_file: url=%s' % url)
 
-        response = requests.head(url)
+        response = self.requests.head(url)
 
         if response.status_code == 404:
             return False
@@ -87,7 +91,7 @@ class RemoteArtifactCache(object):
         logging.debug('RemoteArtifactCache._fetch_file: url=%s' % remote_url)
 
         try:
-            response = requests.get(remote_url, stream=True)
+            response = self.requests.get(remote_url, stream=True)
             response.raise_for_status()
             content_length = int(response.headers.get('content-length', 0))
             for i, chunk in enumerate(response.iter_content(chunk_size)):
diff --git a/morphlib/remoteartifactcache_tests.py b/morphlib/remoteartifactcache_tests.py
index 680bb22d..820958da 100644
--- a/morphlib/remoteartifactcache_tests.py
+++ b/morphlib/remoteartifactcache_tests.py
@@ -16,7 +16,7 @@
 
 import BaseHTTPServer
 import contextlib
-import os
+import logging
 import socket
 import SocketServer
 import threading
@@ -27,26 +27,36 @@ import morphlib
 import morphlib.testutils
 
 
-def artifact_files_for_chunk_source(source):
-    '''Determine what filenames would exist for all artifacts of one source.'''
+class FragileTCPServer(SocketServer.TCPServer):
+    '''A TCP server that will optionally refuse some connections.
 
-    existing_files = set(['%s.meta' % source.cache_key])
-    for a in source.artifacts.itervalues():
-        existing_files.add(a.basename())
-        existing_files.add(a.metadata_basename('meta'))
-    return existing_files
+    This is used for simulating temporary network glitches.
+    '''
+
+    def __init__(self, *args):
+        SocketServer.TCPServer.__init__(self, *args)
+        self._refuse_next_connection = False
+
+    def verify_request(self, request, client_address):
+        if self._refuse_next_connection:
+            self._refuse_next_connection = False
+            return False
+        else:
+            return True
 
+    def refuse_next_connection(self):
+        self._refuse_next_connection = True
 
 
 @contextlib.contextmanager
 def http_server(request_handler):
-    server = SocketServer.TCPServer(('', 0), request_handler)
+    server = FragileTCPServer(('', 0), request_handler)
     thread = threading.Thread(target=server.serve_forever)
     thread.setDaemon(True)
     thread.start()
 
     url = 'http://%s:%s' % (socket.gethostname(), server.server_address[1])
-    yield url
+    yield url, server
 
     server.shutdown()
 
@@ -96,25 +106,30 @@ class RemoteArtifactCacheTests(unittest.TestCase):
     def fake_status_cb(*args, **kwargs):
         pass
 
+    def assert_can_download_artifacts(self, rac, source):
+        '''Assert that downloading artifacts from the remote cache works.'''
+        lac = morphlib.testutils.FakeLocalArtifactCache()
+
+        for artifact in source.artifacts.values():
+            self.assertFalse(lac.has(artifact))
+
+        rac.get_artifacts_for_source(
+            source, lac, self.fake_status_cb)
+        for artifact in source.artifacts.values():
+            self.assertTrue(lac.has(artifact))
+
     def test_artifacts_are_cached(self):
         '''Exercise fetching of artifacts that are cached.'''
-
         chunk = morphlib.testutils.example_chunk()
-        valid_files = artifact_files_for_chunk_source(chunk)
+        valid_files = chunk.files()
 
-        with artifact_cache_server(valid_files) as url:
+        with artifact_cache_server(valid_files) as (url, server):
             rac = morphlib.remoteartifactcache.RemoteArtifactCache(url)
             an_artifact = chunk.artifacts.values()[0]
 
             self.assertTrue(
                 rac.has(an_artifact))
-
-            lac = morphlib.testutils.FakeLocalArtifactCache()
-            self.assertFalse(lac.has(an_artifact))
-
-            rac.get_artifacts_for_source(
-                chunk, lac, self.fake_status_cb)
-            self.assertTrue(lac.has(an_artifact))
+            self.assert_can_download_artifacts(rac, chunk)
 
     def test_artifacts_are_not_cached(self):
         '''Exercise trying to fetch artifacts that the cache doesn't have.'''
@@ -122,18 +137,37 @@ class RemoteArtifactCacheTests(unittest.TestCase):
         chunk = morphlib.testutils.example_chunk()
         valid_files = []
 
-        with artifact_cache_server(valid_files) as url:
+        with artifact_cache_server(valid_files) as (url, server):
             rac = morphlib.remoteartifactcache.RemoteArtifactCache(url)
             an_artifact = chunk.artifacts.values()[0]
 
             self.assertFalse(
                 rac.has(an_artifact))
 
-            lac = morphlib.testutils.FakeLocalArtifactCache()
             with self.assertRaises(morphlib.remoteartifactcache.GetError):
+                lac = morphlib.testutils.FakeLocalArtifactCache()
                 rac.get_artifacts_for_source(
                     chunk, lac, self.fake_status_cb)
 
+    def test_retry_requests(self):
+        '''Prove that requests are retried on failure.'''
+
+        chunk = morphlib.testutils.example_chunk()
+        valid_files = chunk.files()
+
+        log = logging.getLogger('requests.packages.urllib3.connectionpool')
+        log.setLevel(logging.ERROR)
+
+        with artifact_cache_server(valid_files) as (url, server):
+            rac = morphlib.remoteartifactcache.RemoteArtifactCache(url)
+            an_artifact = chunk.artifacts.values()[0]
+
+            server.refuse_next_connection()
+            self.assertTrue(rac.has(an_artifact))
+
+            server.refuse_next_connection()
+            self.assert_can_download_artifacts(rac, chunk)
+
     def test_escapes_pluses_in_request_urls(self):
         rac = morphlib.remoteartifactcache.RemoteArtifactCache(
             'http://example.com')