Add gitlab disable_connection_pool option

Under adverse network conditions, it may be necessary to disable
connection pooling entirely in order to achive reliable interaction
with the remote server.  Add an option for that.

Change-Id: I336fab1197a8fb6e038db24277b6279013224bb0

doc/source/reference/drivers/gitlab.rst

@@ -114,6 +114,13 @@ The supported options in ``zuul.conf`` connections are:
 
       TCP connection keepalive timeout; ``0`` disables.
 
+   .. attr:: disable_connection_pool
+      :default: false
+
+      Connection pooling improves performance and resource usage under
+      normal circumstances, but in adverse network conditions it can
+      be problematic.  Set this to ``true`` to disable.
+
 
 
 Trigger Configuration

releasenotes/notes/gitlab-disable-connection-pool-0ad02ef8028d4994.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    It is now possible to disable connection pooling in the Gitlab
+    driver with :attr:`<gitlab connection>.disable_connection_pool`.
+    This option may be useful under adverse network conditions.

tests/fixtures/zuul-gitlab-driver-no-pool.conf

@@ -0,0 +1,48 @@
+[gearman]
+server=127.0.0.1
+
+[web]
+status_url=http://zuul.example.com/status/#{change.number},{change.patchset}
+
+[merger]
+git_dir=/tmp/zuul-test/git
+git_user_email=zuul@example.com
+git_user_name=zuul
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection gitlab]
+driver=gitlab
+server=gitlab
+api_token=0000000000000000000000000000000000000000
+disable_connection_pool=true
+
+[database]
+dburi=$MYSQL_FIXTURE_DBURI$
+
+[connection gitlab2]
+driver=gitlab
+server=gitlabtwo
+api_token=2222
+cloneurl=http://myusername:2222@gitlab
+
+[connection gitlab3]
+driver=gitlab
+server=gitlabthree
+api_token_name=tokenname3
+api_token=3333
+cloneurl=http://myusername:2222@gitlabthree
+
+[connection gitlab4]
+driver=gitlab
+server=gitlabfour
+api_token_name=tokenname4
+api_token=444
+
+[connection gitlab5]
+driver=gitlab
+server=gitlabfive
+api_token_name=tokenname5
+api_token=555
+cloneurl=http://gitlabfivvve

tests/unit/test_gitlab_driver.py

@@ -1089,3 +1089,44 @@ class TestGitlabUnprotectedBranches(ZuulTestCase):
                                           old_sha=new_sha,
                                           new_sha='0' * 40,
                                           removed_files=['zuul.yaml'])
+
+
+class TestGitlabDriverNoPool(ZuulTestCase):
+    config_file = 'zuul-gitlab-driver-no-pool.conf'
+
+    @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab')
+    def test_merge_request_opened(self):
+
+        description = "This is the\nMR description."
+        A = self.fake_gitlab.openFakeMergeRequest(
+            'org/project', 'master', 'A', description=description)
+        self.fake_gitlab.emitEvent(
+            A.getMergeRequestOpenedEvent(), project='org/project')
+        self.waitUntilSettled()
+
+        self.assertEqual('SUCCESS',
+                         self.getJobFromHistory('project-test1').result)
+
+        self.assertEqual('SUCCESS',
+                         self.getJobFromHistory('project-test2').result)
+
+        job = self.getJobFromHistory('project-test2')
+        zuulvars = job.parameters['zuul']
+        self.assertEqual(str(A.number), zuulvars['change'])
+        self.assertEqual(str(A.sha), zuulvars['patchset'])
+        self.assertEqual('master', zuulvars['branch'])
+        self.assertEquals(f'{self.fake_gitlab._test_baseurl}/'
+                          'org/project/merge_requests/1',
+                          zuulvars['items'][0]['change_url'])
+        self.assertEqual(zuulvars["message"], strings.b64encode(description))
+        self.assertEqual(2, len(self.history))
+        self.assertEqual(2, len(A.notes))
+        self.assertEqual(
+            A.notes[0]['body'], "Starting check jobs.")
+        self.assertThat(
+            A.notes[1]['body'],
+            MatchesRegex(r'.*project-test1.*SUCCESS.*', re.DOTALL))
+        self.assertThat(
+            A.notes[1]['body'],
+            MatchesRegex(r'.*project-test2.*SUCCESS.*', re.DOTALL))
+        self.assertTrue(A.approved)

zuul/driver/gitlab/gitlabconnection.py

@@ -32,6 +32,7 @@ from zuul.connection import CachedBranchConnection, ZKChangeCacheMixin
 from zuul.web.handler import BaseWebController
 from zuul.lib.http import ZuulHTTPAdapter
 from zuul.lib.logutil import get_annotated_logger
+from zuul.lib.config import any_to_bool
 from zuul.exceptions import MergeFailure
 from zuul.model import Branch, Project, Ref, Tag
 from zuul.driver.gitlab.gitlabmodel import GitlabTriggerEvent, MergeRequest
@@ -255,18 +256,34 @@ class GitlabAPIClientException(Exception):
 class GitlabAPIClient():
     log = logging.getLogger("zuul.GitlabAPIClient")
 
-    def __init__(self, baseurl, api_token, keepalive):
+    def __init__(self, baseurl, api_token, keepalive, disable_pool):
-        self.session = requests.Session()
+        self._session = None
-        retry = urllib3.util.Retry(total=8,
+        self._orig_baseurl = baseurl
-                                   backoff_factor=0.1)
-        adapter = ZuulHTTPAdapter(keepalive=keepalive,
-                                  max_retries=retry)
-        self.session.mount(baseurl, adapter)
         self.baseurl = '%s/api/v4' % baseurl
         self.api_token = api_token
+        self.keepalive = keepalive
+        self.disable_pool = disable_pool
         self.headers = {'Authorization': 'Bearer %s' % (
             self.api_token)}
 
+        if not self.disable_pool:
+            self._session = self._makeSession()
+
+    def _makeSession(self):
+        session = requests.Session()
+        retry = urllib3.util.Retry(total=8,
+                                   backoff_factor=0.1)
+        adapter = ZuulHTTPAdapter(keepalive=self.keepalive,
+                                  max_retries=retry)
+        session.mount(self._orig_baseurl, adapter)
+        return session
+
+    @property
+    def session(self):
+        if self.disable_pool:
+            return self._makeSession()
+        return self._session
+
     def _manage_error(self, data, code, url, verb, zuul_event_id=None):
         if code < 400:
             return
@@ -442,9 +459,11 @@ class GitlabConnection(ZKChangeCacheMixin, CachedBranchConnection):
         self.api_token = self.connection_config.get(
             'api_token', '')
         self.keepalive = self.connection_config.get('keepalive', 60)
+        self.disable_pool = any_to_bool(self.connection_config.get(
+            'disable_connection_pool', False))
 
         self.gl_client = GitlabAPIClient(self.baseurl, self.api_token,
-                                         self.keepalive)
+                                         self.keepalive, self.disable_pool)
         self.sched = None
         self.source = driver.getSource(self)
 

zuul/lib/config.py

@@ -28,3 +28,10 @@ def get_default(config, section, option, default=None, expand_user=False):
     if expand_user and value:
         return os.path.expanduser(value)
     return value
+
+
+def any_to_bool(val):
+    val = str(val)
+    if val.lower() in ('1', 'on', 'yes', 'true'):
+        return True
+    return False