Add TCP keepalive to gitlab

The gitlab driver uses requests to interact with gitlab's API.  By
default, requests uses a connection pool to cache and re-use HTTP
connections. If the gitlab server is across a network segment with,
say, a poor NAT implementation which silently drops idle connections
from its map, then Zuul may not notice until it attempts to make a
request on one of these connections and receives a read timeout.

To avoid this, use TCP keepalives on these HTTP requests by default.

This is configured to send a keepalive packet by default every 60
seconds.  This should avoid sending too much extra traffic, while
improving the reliability for all users.  The default of 60 seconds
was chosen to match the SSH keepalive value we set for gerrit
connections (for much the same reason).

If this works well, we can use this approach with other requests-based
drivers (and at this point, every driver uses requests to at least
some degree).

Change-Id: I86ce064c6ec9325e2ae9db516778058050696ef6

4 files changed, 56 insertions, 7 deletions

diff --git a/doc/source/reference/drivers/gitlab.rst b/doc/source/reference/drivers/gitlab.rst
index 4bbd21a49..5e1c15668 100644
--- a/doc/source/reference/drivers/gitlab.rst
+++ b/doc/source/reference/drivers/gitlab.rst
@@ -101,7 +101,7 @@ The supported options in ``zuul.conf`` connections are:
 
    .. attr:: cloneurl
       :default: {baseurl}
-      
+
       Omit to clone using http(s) or set to ``ssh://git@{server}``.
       If **api_token_name** is set and **cloneurl** is either omitted or is
       set without credentials, **cloneurl** will be modified to use credentials
@@ -109,6 +109,12 @@ The supported options in ``zuul.conf`` connections are:
       If **cloneurl** is defined with credentials, it will be used as is,
       without modification from the driver.
 
+   .. attr:: keepalive
+      :default: 60
+
+      TCP connection keepalive timeout; ``0`` disables.
+
+
 
 Trigger Configuration
 ---------------------
diff --git a/tests/base.py b/tests/base.py
index d0c5fb16c..c09cd99c9 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -1927,7 +1927,7 @@ class FakeGitlabConnection(gitlabconnection.GitlabConnection):
                                                    connection_config)
         self.merge_requests = changes_db
         self.gl_client = FakeGitlabAPIClient(
-            self.baseurl, self.api_token, merge_requests_db=changes_db)
+            self.baseurl, self.api_token, 60, merge_requests_db=changes_db)
         self.rpcclient = rpcclient
         self.upstream_root = upstream_root
         self.mr_number = 0
@@ -2024,8 +2024,10 @@ FakeBranch = namedtuple('Branch', ('name', 'protected'))
 class FakeGitlabAPIClient(gitlabconnection.GitlabAPIClient):
     log = logging.getLogger("zuul.test.FakeGitlabAPIClient")
 
-    def __init__(self, baseurl, api_token, merge_requests_db={}):
-        super(FakeGitlabAPIClient, self).__init__(baseurl, api_token)
+    def __init__(self, baseurl, api_token, keepalive,
+                 merge_requests_db={}):
+        super(FakeGitlabAPIClient, self).__init__(
+            baseurl, api_token, keepalive)
         self.merge_requests = merge_requests_db
         self.fake_repos = defaultdict(lambda: IterableList('name'))
         self.community_edition = False
diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py
index 11edd1094..9a6cbc636 100644
--- a/zuul/driver/gitlab/gitlabconnection.py
+++ b/zuul/driver/gitlab/gitlabconnection.py
@@ -30,6 +30,7 @@ from typing import List, Optional
 
 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.exceptions import MergeFailure
 from zuul.model import Branch, Project, Ref, Tag
@@ -254,11 +255,12 @@ class GitlabAPIClientException(Exception):
 class GitlabAPIClient():
     log = logging.getLogger("zuul.GitlabAPIClient")
 
-    def __init__(self, baseurl, api_token):
+    def __init__(self, baseurl, api_token, keepalive):
         self.session = requests.Session()
         retry = urllib3.util.Retry(total=8,
                                    backoff_factor=0.1)
-        adapter = requests.adapters.HTTPAdapter(max_retries=retry)
+        adapter = ZuulHTTPAdapter(keepalive=keepalive,
+                                  max_retries=retry)
         self.session.mount(baseurl, adapter)
         self.baseurl = '%s/api/v4' % baseurl
         self.api_token = api_token
@@ -435,7 +437,10 @@ class GitlabConnection(ZKChangeCacheMixin, CachedBranchConnection):
             'api_token_name', '')
         self.api_token = self.connection_config.get(
             'api_token', '')
-        self.gl_client = GitlabAPIClient(self.baseurl, self.api_token)
+        self.keepalive = self.connection_config.get('keepalive', 60)
+
+        self.gl_client = GitlabAPIClient(self.baseurl, self.api_token,
+                                         self.keepalive)
         self.sched = None
         self.source = driver.getSource(self)
 
diff --git a/zuul/lib/http.py b/zuul/lib/http.py
new file mode 100644
index 000000000..1b74e5942
--- /dev/null
+++ b/zuul/lib/http.py
@@ -0,0 +1,36 @@
+# Copyright 2021 Acme Gating, LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import socket
+
+from requests.adapters import HTTPAdapter
+from urllib3.connection import HTTPConnection
+
+
+class ZuulHTTPAdapter(HTTPAdapter):
+    "A requests HTTPAdapter class which supports TCP keepalives"
+    def __init__(self, *args, **kw):
+        self.keepalive = int(kw.pop('keepalive', 0))
+        super().__init__(*args, **kw)
+
+    def init_poolmanager(self, *args, **kw):
+        if self.keepalive:
+            ka = self.keepalive
+            kw['socket_options'] = HTTPConnection.default_socket_options + [
+                (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),
+                (socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, ka),
+                (socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, ka),
+                (socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 2),
+            ]
+        return super().init_poolmanager(*args, **kw)