diff --git a/doc/source/reference/drivers/gitlab.rst b/doc/source/reference/drivers/gitlab.rst index 5e1c156685..c747bbbd3f 100644 --- a/doc/source/reference/drivers/gitlab.rst +++ b/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 diff --git a/releasenotes/notes/gitlab-disable-connection-pool-0ad02ef8028d4994.yaml b/releasenotes/notes/gitlab-disable-connection-pool-0ad02ef8028d4994.yaml new file mode 100644 index 0000000000..4aadd4ee30 --- /dev/null +++ b/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:`.disable_connection_pool`. + This option may be useful under adverse network conditions. diff --git a/tests/fixtures/zuul-gitlab-driver-no-pool.conf b/tests/fixtures/zuul-gitlab-driver-no-pool.conf new file mode 100644 index 0000000000..7cf788e1ac --- /dev/null +++ b/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 diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index d395216a84..1444f9a9be 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/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) diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index 7589474c8c..d409815622 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/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): - self.session = requests.Session() - retry = urllib3.util.Retry(total=8, - backoff_factor=0.1) - adapter = ZuulHTTPAdapter(keepalive=keepalive, - max_retries=retry) - self.session.mount(baseurl, adapter) + def __init__(self, baseurl, api_token, keepalive, disable_pool): + self._session = None + self._orig_baseurl = baseurl 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) diff --git a/zuul/lib/config.py b/zuul/lib/config.py index 867e8a789e..a8bf2ead2d 100644 --- a/zuul/lib/config.py +++ b/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