Set a timeout for all data fetches using requests

A timeout config value is added for each collector which uses requests
to fetch data, and this value is used for any requests calls.

Without a timeout a request may stall indefinitely and
os-collect-config will stop polling.

A timeout default of 10 seconds is chosen as the default. This is used
for both the connection timeout and the read timeout.

Change-Id: I4ad0065b5a85393105c6385a15653d7204b4f880
Closes-Bug: #1600652

os_collect_config/cfn.py

@@ -48,7 +48,10 @@ opts = [
     cfg.MultiStrOpt('deployment-key',
                     default=['deployments'],
                     help='DEPRECATED, use global configuration option '
-                         '"deployment-key"')
+                         '"deployment-key"'),
+    cfg.FloatOpt('timeout', default=10,
+                 help='Seconds to wait for the connection and read request'
+                      ' timeout.')
 ]
 name = 'cfn'
 
@@ -107,7 +110,8 @@ class Collector(object):
             try:
                 content = self._session.get(
                     url, params=params, headers=headers,
-                    verify=CONF.cfn.ca_certificate)
+                    verify=CONF.cfn.ca_certificate,
+                    timeout=CONF.cfn.timeout)
                 content.raise_for_status()
             except self._requests_impl.exceptions.RequestException as e:
                 logger.warn(e)

os_collect_config/ec2.py

@@ -25,7 +25,10 @@ CONF = cfg.CONF
 opts = [
     cfg.StrOpt('metadata-url',
                default=EC2_METADATA_URL,
-               help='URL to query for EC2 Metadata')
+               help='URL to query for EC2 Metadata'),
+    cfg.FloatOpt('timeout', default=10,
+                 help='Seconds to wait for the connection and read request'
+                      ' timeout.')
 ]
 name = 'ec2'
 
@@ -35,9 +38,9 @@ class Collector(object):
         self._requests_impl = requests_impl
         self.session = requests_impl.Session()
 
-    def _fetch_metadata(self, fetch_url):
+    def _fetch_metadata(self, fetch_url, timeout):
         try:
-            r = self.session.get(fetch_url)
+            r = self.session.get(fetch_url, timeout=timeout)
             r.raise_for_status()
         except self._requests_impl.exceptions.RequestException as e:
             log.getLogger(__name__).warn(e)
@@ -51,10 +54,11 @@ class Collector(object):
                 sub_fetch_url = fetch_url + subkey
                 if subkey[-1] == '/':
                     subkey = subkey[:-1]
-                new_content[subkey] = self._fetch_metadata(sub_fetch_url)
+                new_content[subkey] = self._fetch_metadata(
+                    sub_fetch_url, timeout)
             content = new_content
         return content
 
     def collect(self):
         root_url = '%s/' % (CONF.ec2.metadata_url)
-        return [('ec2', self._fetch_metadata(root_url))]
+        return [('ec2', self._fetch_metadata(root_url, CONF.ec2.timeout))]

os_collect_config/request.py

@@ -30,6 +30,9 @@ logger = log.getLogger(__name__)
 opts = [
     cfg.StrOpt('metadata-url',
                help='URL to query for metadata'),
+    cfg.FloatOpt('timeout', default=10,
+                 help='Seconds to wait for the connection and read request'
+                      ' timeout.')
 ]
 name = 'request'
 
@@ -70,13 +73,14 @@ class Collector(object):
             logger.info('No metadata_url configured.')
             raise exc.RequestMetadataNotConfigured
         url = CONF.request.metadata_url
+        timeout = CONF.request.timeout
         final_content = {}
 
         try:
-            head = self._session.head(url)
+            head = self._session.head(url, timeout=timeout)
             last_modified = self.check_fetch_content(head.headers)
 
-            content = self._session.get(url)
+            content = self._session.get(url, timeout=timeout)
             content.raise_for_status()
             self.last_modified = last_modified
 

os_collect_config/tests/test_cfn.py

@@ -125,7 +125,7 @@ class FakeReqSession(object):
         self._expected_netloc = expected_netloc
         self.verify = False
 
-    def get(self, url, params, headers, verify=None):
+    def get(self, url, params, headers, verify=None, timeout=None):
         self._test.addDetail('url', test_content.text_content(url))
         url = urlparse.urlparse(url)
         self._test.assertEqual(self._expected_netloc, url.netloc)
@@ -140,6 +140,7 @@ class FakeReqSession(object):
                                params['Action'])
         self._test.assertIn('LogicalResourceId', params)
         self._test.assertEqual('foo', params['LogicalResourceId'])
+        self._test.assertEqual(10, timeout)
         root = etree.Element('DescribeStackResourceResponse')
         result = etree.SubElement(root, 'DescribeStackResourceResult')
         detail = etree.SubElement(result, 'StackResourceDetail')
@@ -189,7 +190,7 @@ class FakeFailRequests(object):
     exceptions = requests.exceptions
 
     class Session(object):
-        def get(self, url, params, headers, verify=None):
+        def get(self, url, params, headers, verify=None, timeout=None):
             raise requests.exceptions.HTTPError(403, 'Forbidden')
 
 

os_collect_config/tests/test_ec2.py

@@ -63,7 +63,7 @@ class FakeRequests(object):
     exceptions = requests.exceptions
 
     class Session(object):
-        def get(self, url):
+        def get(self, url, timeout=None):
             url = urlparse.urlparse(url)
 
             if url.path == '/latest/meta-data/':
@@ -81,7 +81,7 @@ class FakeFailRequests(object):
     exceptions = requests.exceptions
 
     class Session(object):
-        def get(self, url):
+        def get(self, url, timeout=None):
             raise requests.exceptions.HTTPError(403, 'Forbidden')
 
 

os_collect_config/tests/test_request.py

@@ -111,10 +111,10 @@ class FakeRequests(object):
     exceptions = requests.exceptions
 
     class Session(object):
-        def get(self, url):
+        def get(self, url, timeout=None):
             return FakeResponse(json.dumps(META_DATA))
 
-        def head(self, url):
+        def head(self, url, timeout=None):
             return FakeResponse('', headers={
                 'last-modified': time.strftime(
                     "%a, %d %b %Y %H:%M:%S %Z", time.gmtime())})
@@ -124,20 +124,20 @@ class FakeFailRequests(object):
     exceptions = requests.exceptions
 
     class Session(object):
-        def get(self, url):
+        def get(self, url, timeout=None):
             raise requests.exceptions.HTTPError(403, 'Forbidden')
 
-        def head(self, url):
+        def head(self, url, timeout=None):
             raise requests.exceptions.HTTPError(403, 'Forbidden')
 
 
 class FakeRequestsSoftwareConfig(object):
 
     class Session(object):
-        def get(self, url):
+        def get(self, url, timeout=None):
             return FakeResponse(json.dumps(SOFTWARE_CONFIG_DATA))
 
-        def head(self, url):
+        def head(self, url, timeout=None):
             return FakeResponse('', headers={
                 'last-modified': time.strftime(
                     "%a, %d %b %Y %H:%M:%S %Z", time.gmtime())})