ERP5: enable publisher-timeout by default

 - enable publisher-timeout by default, after 300s like haproxy used to do
 - make haproxy use similar timeout as publisher-timeout
 - small fixes to json schemas

See merge request nexedi/slapos!1192

software/erp5/instance-erp5-input-schema.json

@@ -97,7 +97,7 @@
     },
     "publisher-timeout": {
       "description": "How long a publisher-initiated transaction may last, in seconds",
-      "default": null,
+      "default": 300,
       "type": [
         "number",
         "null"
@@ -339,6 +339,7 @@
     },
     "zodb-zeo": {
       "description": "Common settings ZEO servers",
+      "additionalProperties": false,
       "properties": {
         "tcpv4-port": {
           "allOf": [
@@ -464,6 +465,7 @@
     },
     "jupyter": {
       "description": "Jupyter subinstance parameters",
+      "additionalProperties": false,
       "properties": {
         "enable": {
           "description": "Whether to enable creation of associated Jupyter subinstance",
@@ -480,6 +482,7 @@
     },
     "wcfs": {
       "description": "Parameters for wendelin.core filesystem",
+      "additionalProperties": false,
       "properties": {
         "enable": {
           "description": "Whether to enable WCFS filesystem and use it to access ZBigArray/ZBigFile data. In WCFS mode wendelin.core clients (Zope/ERP5 processes) share in-RAM cache for in-ZODB data without duplicating it for every client. This cache sharing does not affect correctness as isolation property is continued to be provided to every client.",
@@ -495,22 +498,27 @@
     },
     "caucase": {
       "description": "Caucase certificate authority parameters",
-      "properties": {
-        "url": {
-          "title": "Caucase URL",
-          "description": "URL of existing caucase instance to use. If empty, a new caucase instance will be deployed. If not empty, other properties in this section will be ignored.",
-          "default": "",
-          "type": "string",
-          "format": "uri"
+      "allOf": [
+        {
+          "properties": {
+            "url": {
+              "title": "Caucase URL",
+              "description": "URL of existing caucase instance to use. If empty, a new caucase instance will be deployed. If not empty, other properties in this section will be ignored.",
+              "default": "",
+              "type": "string",
+              "format": "uri"
+            }
+          }
+        },
+        {
+          "$ref": "../caucase/instance-caucase-input-schema.json"
         }
-      },
-      "additionalProperties": {
-        "$ref": "../caucase/instance-caucase-input-schema.json"
-      },
+      ],
       "type": "object"
     },
     "test-runner": {
       "description": "Test runner parameters.",
+      "additionalProperties": false,
       "properties": {
         "enabled": {
           "description": "Generate helper scripts to run test suite.",
@@ -650,6 +658,7 @@
         },
         "ssl": {
           "description": "HTTPS certificate generation parameters",
+          "additionalProperties": false,
           "properties": {
             "frontend-caucase-url-list": {
               "title": "Frontend Caucase URL List",

software/erp5/instance-kumofs-schema.json

 {
   "$schema": "http://json-schema.org/draft-04/schema#",
   "type": "object",
+  "additionalProperties": false,
   "properties": {
     "tcpv4-port": {
       "allOf": [

software/erp5/instance-mariadb-schema.json

 {
   "$schema": "http://json-schema.org/draft-04/schema#",
   "type": "object",
+  "additionalProperties": false,
   "properties": {
     "tcpv4-port": {
       "allOf": [

software/erp5/instance-smtp-schema.json

 {
   "$schema": "http://json-schema.org/draft-04/schema#",
   "type": "object",
+  "additionalProperties": false,
   "properties": {
     "tcpv4-port": {
       "allOf": [

software/erp5/test/test/test_balancer.py

@@ -166,10 +166,11 @@ class BalancerTestCase(ERP5InstanceTestCase):
         'backend-path-dict': {
             'default': '',
         },
-        'ssl-authentication-dict': {},
+        'ssl-authentication-dict': {'default': False},
         'ssl': {
             'caucase-url': cls.getManagedResource("caucase", CaucaseService).url,
         },
+        'timeout-dict': {'default': None},
         'family-path-routing-dict': {},
         'path-routing-list': [],
       }
@@ -186,18 +187,51 @@ class BalancerTestCase(ERP5InstanceTestCase):
 
 
 class SlowHTTPServer(ManagedHTTPServer):
-  """An HTTP Server which reply after 2 seconds.
+  """An HTTP Server which reply after a timeout.
+
+  Timeout is 2 seconds by default, and can be specified in the path of the URL
   """
   class RequestHandler(BaseHTTPRequestHandler):
     def do_GET(self):
       # type: () -> None
       self.send_response(200)
       self.send_header("Content-Type", "text/plain")
-      time.sleep(2)
+      timeout = 2
+      try:
+        timeout = int(self.path[1:])
+      except ValueError:
+        pass
+      time.sleep(timeout)
       self.end_headers()
       self.wfile.write(b"OK\n")
 
-    log_message = logging.getLogger(__name__ + '.SlowHandler').info
+    log_message = logging.getLogger(__name__ + '.SlowHTTPServer').info
+
+
+class TestTimeout(BalancerTestCase, CrontabMixin):
+  __partition_reference__ = 't'
+  @classmethod
+  def _getInstanceParameterDict(cls):
+    # type: () -> dict
+    parameter_dict = super(TestTimeout, cls)._getInstanceParameterDict()
+    # use a slow server instead
+    parameter_dict['dummy_http_server'] = [[cls.getManagedResource("slow_web_server", SlowHTTPServer).netloc, 1, False]]
+    # and set timeout of 1 second
+    parameter_dict['timeout-dict'] = {'default': 1}
+    return parameter_dict
+
+  def test_timeout(self):
+    # type: () -> None
+    self.assertEqual(
+      requests.get(
+          six.moves.urllib.parse.urljoin(self.default_balancer_url, '/1'),
+          verify=False).status_code,
+      requests.codes.ok)
+    self.assertEqual(
+      requests.get(
+          six.moves.urllib.parse.urljoin(self.default_balancer_url, '/5'),
+          verify=False).status_code,
+      requests.codes.gateway_timeout)
 
 
 class TestLog(BalancerTestCase, CrontabMixin):
@@ -753,6 +787,7 @@ class TestFrontendXForwardedFor(BalancerTestCase):
         'default': False,
         'default-auth': True,
     }
+    parameter_dict['timeout-dict']['default-auth'] = None
     parameter_dict['ssl']['frontend-caucase-url-list'] = [frontend_caucase.url]
     return parameter_dict
 
@@ -926,6 +961,8 @@ class TestPathBasedRouting(BalancerTestCase):
     ] = parameter_dict['zope-family-dict'][
       'default'
     ]
+    parameter_dict['timeout-dict']['second'] = None
+    parameter_dict['ssl-authentication-dict']['second'] = False
     # Routing rules outermost slashes mean nothing. They are internally
     # stripped and rebuilt in order to correctly represent the request's URL.
     parameter_dict['family-path-routing-dict'] = {

software/erp5/test/test/test_erp5.py

@@ -406,7 +406,7 @@ class TestWatchActivities(ERP5InstanceTestCase):
   __partition_reference__ = 'wa'
 
   def test(self):
-    # "watch_activites" scripts use watch command. We'll fake a watch command
+    # "watch_activities" scripts use watch command. We'll fake a watch command
     # that executes the actual command only once to check the output.
     tmpdir = tempfile.mkdtemp()
     self.addCleanup(shutil.rmtree, tmpdir)
@@ -443,12 +443,86 @@ class TestWatchActivities(ERP5InstanceTestCase):
     self.assertIn(' dict ', output)
 
 
-class ZopeTestMixin(CrontabMixin):
+class ZopeSkinsMixin(object):
+  """Mixins with utility methods to test zope behaviors.
+  """
+  @classmethod
+  def _setUpClass(cls):
+    super(ZopeSkinsMixin, cls)._setUpClass()
+    param_dict = cls.getRootPartitionConnectionParameterDict()
+    with cls.getXMLRPCClient() as erp5_xmlrpc_client:
+      # wait for ERP5 to be ready (TODO: this should probably be a promise)
+      for _ in range(120):
+        time.sleep(1)
+        try:
+          erp5_xmlrpc_client.getTitle()
+        except (six.moves.xmlrpc_client.ProtocolError,
+                six.moves.xmlrpc_client.Fault):
+          pass
+        else:
+          break
+
+  @classmethod
+  def _getAuthenticatedZopeUrl(cls, path, family_name='default'):
+    """Returns a URL to access a zope family through balancer,
+    with credentials in the URL.
+
+    path is joined with urllib.parse.urljoin to the URL of the portal.
+    """
+    param_dict = cls.getRootPartitionConnectionParameterDict()
+    parsed = six.moves.urllib.parse.urlparse(param_dict['family-' + family_name])
+    base_url = parsed._replace(
+        netloc='{}:{}@{}:{}'.format(
+            param_dict['inituser-login'],
+            param_dict['inituser-password'],
+            parsed.hostname,
+            parsed.port,
+        ),
+        path=param_dict['site-id'] + '/',
+    ).geturl()
+    return six.moves.urllib_parse.urljoin(base_url, path)
+
+  @classmethod
+  @contextlib.contextmanager
+  def getXMLRPCClient(cls):
+    # don't verify certificate
+    ssl_context = ssl.create_default_context()
+    ssl_context.check_hostname = False
+    ssl_context.verify_mode = ssl.CERT_NONE
+    erp5_xmlrpc_client = six.moves.xmlrpc_client.ServerProxy(
+        cls._getAuthenticatedZopeUrl(''),
+        context=ssl_context,
+    )
+    # BBB use as a context manager only on python3
+    if sys.version_info < (3, ):
+      yield erp5_xmlrpc_client
+    else:
+      with erp5_xmlrpc_client:
+        yield erp5_xmlrpc_client
+
+  @classmethod
+  def _addPythonScript(cls, script_id, params, body):
+    with cls.getXMLRPCClient() as erp5_xmlrpc_client:
+      custom = erp5_xmlrpc_client.portal_skins.custom
+      try:
+        custom.manage_addProduct.PythonScripts.manage_addPythonScript(
+            script_id)
+      except six.moves.xmlrpc_client.ProtocolError as e:
+        if e.errcode != 302:
+          raise
+      getattr(custom, script_id).ZPythonScriptHTML_editAction(
+          '',
+          '',
+          params,
+          body,
+      )
+
+
+class ZopeTestMixin(ZopeSkinsMixin, CrontabMixin):
   """Mixin class for zope features.
   """
   wsgi = NotImplemented # type: bool
 
-
   __partition_reference__ = 'z'
 
   @classmethod
@@ -469,74 +543,14 @@ class ZopeTestMixin(CrontabMixin):
   @classmethod
   def _setUpClass(cls):
     super(ZopeTestMixin, cls)._setUpClass()
-
+    cls.zope_base_url = cls._getAuthenticatedZopeUrl('')
     param_dict = cls.getRootPartitionConnectionParameterDict()
-    # rebuild an url with user and password
-    parsed = six.moves.urllib.parse.urlparse(param_dict['family-default'])
-    cls.zope_base_url = parsed._replace(
-        netloc='{}:{}@{}:{}'.format(
-            param_dict['inituser-login'],
-            param_dict['inituser-password'],
-            parsed.hostname,
-            parsed.port,
-        ),
-        path=param_dict['site-id'] + '/',
-    ).geturl()
-
-    cls.zope_deadlock_debugger_url = six.moves.urllib_parse.urljoin(
-        cls.zope_base_url,
+    cls.zope_deadlock_debugger_url = cls._getAuthenticatedZopeUrl(
         '/manage_debug_threads?{deadlock-debugger-password}'.format(
-            **param_dict),
-    )
-
-
-    @contextlib.contextmanager
-    def getXMLRPCClient():
-      # don't verify certificate
-      ssl_context = ssl.create_default_context()
-      ssl_context.check_hostname = False
-      ssl_context.verify_mode = ssl.CERT_NONE
-      erp5_xmlrpc_client = six.moves.xmlrpc_client.ServerProxy(
-          cls.zope_base_url,
-          context=ssl_context,
-      )
-      # BBB use as a context manager only on python3
-      if sys.version_info < (3, ):
-        yield erp5_xmlrpc_client
-      else:
-        with erp5_xmlrpc_client:
-          yield erp5_xmlrpc_client
-
-    with getXMLRPCClient() as erp5_xmlrpc_client:
-      # wait for ERP5 to be ready (TODO: this should probably be a promise)
-      for _ in range(120):
-        time.sleep(1)
-        try:
-          erp5_xmlrpc_client.getTitle()
-        except (six.moves.xmlrpc_client.ProtocolError,
-                six.moves.xmlrpc_client.Fault):
-          pass
-        else:
-          break
-
-    def addPythonScript(script_id, params, body):
-      with getXMLRPCClient() as erp5_xmlrpc_client:
-        custom = erp5_xmlrpc_client.portal_skins.custom
-        try:
-          custom.manage_addProduct.PythonScripts.manage_addPythonScript(
-              script_id)
-        except six.moves.xmlrpc_client.ProtocolError as e:
-          if e.errcode != 302:
-            raise
-        getattr(custom, script_id).ZPythonScriptHTML_editAction(
-            '',
-            '',
-            params,
-            body,
-        )
+        **param_dict))
 
     # a python script to verify activity processing
-    addPythonScript(
+    cls._addPythonScript(
         script_id='ERP5Site_verifyActivityProcessing',
         params='mode',
         body='''if 1:
@@ -556,7 +570,7 @@ class ZopeTestMixin(CrontabMixin):
         'ERP5Site_verifyActivityProcessing',
     )
     # a python script logging to event log
-    addPythonScript(
+    cls._addPythonScript(
         script_id='ERP5Site_logMessage',
         params='name',
         body='''if 1:
@@ -569,14 +583,14 @@ class ZopeTestMixin(CrontabMixin):
         'ERP5Site_logMessage',
     )
     # a python script issuing a long request
-    addPythonScript(
+    cls._addPythonScript(
         script_id='ERP5Site_executeLongRequest',
         params='',
         body='''if 1:
           import time
           for _ in range(5):
             time.sleep(1)
-          return "done"    
+          return "done"
         ''',
     )
     cls.zope_long_request_url = six.moves.urllib_parse.urljoin(
@@ -850,3 +864,61 @@ class TestZopeWSGI(ZopeTestMixin, ERP5InstanceTestCase):
   @unittest.expectedFailure
   def test_basic_authentication_user_in_access_log(self):
     super(TestZopeWSGI, self).test_basic_authentication_user_in_access_log(self)
+
+
+class TestZopePublisherTimeout(ZopeSkinsMixin, ERP5InstanceTestCase):
+  __partition_reference__ = 't'
+
+  @classmethod
+  def getInstanceParameterDict(cls):
+    return {
+        '_':
+        json.dumps({
+            # a default timeout of 3
+            "publisher-timeout": 3,
+            # and a family without timeout
+            "family-override": {
+                "no-timeout": {
+                    "publisher-timeout": None,
+                },
+            },
+            "zope-partition-dict": {
+                "default": {
+                    "family": "default",
+                },
+                "no-timeout": {
+                    "family": "no-timeout",
+                    "port-base": 2300,
+                },
+            },
+        })
+    }
+
+  @classmethod
+  def _setUpClass(cls):
+    super(TestZopePublisherTimeout, cls)._setUpClass()
+    cls._addPythonScript(
+      'ERP5Site_doSlowRequest',
+      '',
+      '''if 1:
+        import time
+        def recurse(o):
+          time.sleep(0.1)
+          for sub in o.objectValues():
+            recurse(sub)
+        recurse(context.getPortalObject())
+      '''
+    )
+
+  def test_long_request_interupted_on_default_family(self):
+    ret = requests.get(self._getAuthenticatedZopeUrl(
+      'ERP5Site_doSlowRequest', family_name='default'), verify=False)
+    self.assertIn('TimeoutReachedError', ret.text)
+    self.assertEqual(ret.status_code, requests.codes.server_error)
+
+  def test_long_request_not_interupted_on_no_timeout_family(self):
+    with self.assertRaises(requests.exceptions.Timeout):
+      requests.get(
+        self._getAuthenticatedZopeUrl('ERP5Site_doSlowRequest', family_name='no-timeout'),
+        verify=False,
+        timeout=6)

stack/erp5/buildout.hash.cfg

@@ -74,7 +74,7 @@ md5sum = 9a7f7888ba4183c9d900e862074f3baf
 
 [template-erp5]
 filename = instance-erp5.cfg.in
-md5sum = f5a1661449c9681b3de7d4af645124ba
+md5sum = 3d8f3a440b7423c3b947c6ea4d775c6e
 
 [template-zeo]
 filename = instance-zeo.cfg.in
@@ -90,11 +90,11 @@ md5sum = c3e3f8cd985407931b705d15bdedc8d9
 
 [template-balancer]
 filename = instance-balancer.cfg.in
-md5sum = 4491b256241cb4bbdfafb22ae95a3eba
+md5sum = b0751d3d12cfcc8934cb1027190f5e5e
 
 [template-haproxy-cfg]
 filename = haproxy.cfg.in
-md5sum = d2d98ed3fafce764991b72371e3e09d5
+md5sum = 1645ef8990ab2b50f91a4c02f0cf8882
 
 [template-rsyslogd-cfg]
 filename = rsyslogd.cfg.in

stack/erp5/haproxy.cfg.in

@@ -48,6 +48,7 @@
  #            ( 8000, # port int
  #              'https', # proto str
  #               True, # ssl_required bool
+ #               None,  # timeout (in seconds) int | None
  #               [  # backends
  #                  '10.0.0.10:8001', # netloc str
  #                   1, # max_connection_count int
@@ -59,6 +60,7 @@
  #            ( 8002, # port int
  #              'https', # proto str
  #               False, # ssl_required bool
+ #               None,  # timeout (in seconds) int | None
  #               [  # backends
  #                  '10.0.0.10:8003', # netloc str
  #                   1, # max_connection_count int
@@ -135,7 +137,6 @@ defaults
 
   timeout connect 10s
   timeout queue 60s
-  timeout server 305s
   timeout client 305s
 
   option http-server-close
@@ -150,7 +151,7 @@ defaults
 {% set family_path_routing_dict = parameter_dict['family-path-routing-dict'] %}
 {% set path_routing_list = parameter_dict['path-routing-list'] %}
 
-{% for name, (port, _, certificate_authentication, backend_list) in sorted(six.iteritems(parameter_dict['backend-dict'])) -%}
+{% for name, (port, _, certificate_authentication, timeout, backend_list) in sorted(six.iteritems(parameter_dict['backend-dict'])) -%}
 listen family_{{ name }}
 {%-  if parameter_dict.get('ca-cert') -%}
 {%-    set ssl_auth = ' ca-file ' ~ parameter_dict['ca-cert'] ~ ' verify' ~ ( ' required' if certificate_authentication else ' optional' ) ~ ' crl-file ' ~ parameter_dict['crl'] %}
@@ -162,6 +163,15 @@ listen family_{{ name }}
   cookie SERVERID rewrite
   http-request set-header X-Balancer-Current-Cookie SERVERID
 
+{% if timeout %}
+  {#
+    Apply a slightly longer timeout than the zope timeout so that clients can see the
+    TimeoutReachedError from zope, that is a bit more informative than the 504 error
+    page from haproxy.
+  #}
+  timeout server {{ timeout + 3 }}s
+{%-  endif %}
+
   # remove X-Forwarded-For unless client presented a verified certificate
   acl client_cert_verified ssl_c_used ssl_c_verify 0
   http-request del-header X-Forwarded-For unless client_cert_verified

stack/erp5/instance-balancer.cfg.in

@@ -123,7 +123,7 @@ update-command = ${:command}
 {% for family_name, parameter_id_list in sorted(
   six.iteritems(slapparameter_dict['zope-family-dict'])) -%}
 {%   set zope_family_address_list = [] -%}
-{%   set ssl_authentication = slapparameter_dict['ssl-authentication-dict'].get(family_name, False) -%}
+{%   set ssl_authentication = slapparameter_dict['ssl-authentication-dict'][family_name] -%}
 {%   set has_webdav = [] -%}
 {%   for parameter_id in parameter_id_list -%}
 {%     set zope_address_list = slapparameter_dict[parameter_id] -%}
@@ -177,7 +177,7 @@ update-command = ${:command}
 {%   else %}
 {%     set external_scheme = 'https' -%}
 {%   endif -%}
-{%   do haproxy_dict.__setitem__(family_name, (haproxy_port, external_scheme, slapparameter_dict['ssl-authentication-dict'].get(family_name, False), zope_family_address_list)) -%}
+{%   do haproxy_dict.__setitem__(family_name, (haproxy_port, external_scheme, slapparameter_dict['ssl-authentication-dict'][family_name], slapparameter_dict['timeout-dict'][family_name], zope_family_address_list)) -%}
 {% endfor -%}
 
 [haproxy-cfg-parameter-dict]
@@ -309,7 +309,7 @@ config-port = {{ next(six.itervalues(haproxy_dict))[0] }}
 
 [{{ section('publish') }}]
 recipe = slapos.cookbook:publish.serialised
-{% for family_name, (port, scheme, _, _) in haproxy_dict.items() -%}
+{% for family_name, (port, scheme, _, _, _) in haproxy_dict.items() -%}
 {{   family_name ~ '-v6' }} = {% if ipv6_set %}{{ scheme ~ '://[' ~ ipv6 ~ ']:' ~ port }}{% endif %}
 {{   family_name }} = {{ scheme ~ '://' ~ ipv4 ~ ':' ~ port }}
 {% endfor -%}

stack/erp5/instance-erp5.cfg.in

@@ -265,12 +265,13 @@ config-wcfs_enable = {{ dumps(wcfs_enable) }}
 config-test-runner-configuration = {{ dumps(slapparameter_dict.get('test-runner', {})) }}
 software-type = zope
 
-{% set global_publisher_timeout = slapparameter_dict.get('publisher-timeout') -%}
+{% set global_publisher_timeout = slapparameter_dict.get('publisher-timeout', 300) -%}
 {% set global_activity_timeout = slapparameter_dict.get('activity-timeout') -%}
 {% set zope_family_dict = {} -%}
 {% set zope_family_name_list = [] -%}
 {% set zope_backend_path_dict = {} -%}
 {% set ssl_authentication_dict = {} -%}
+{% set balancer_timeout_dict = {} -%}
 {% set jupyter_zope_family_default = [] -%}
 {% for custom_name, zope_parameter_dict in six.iteritems(zope_partition_dict) -%}
 {%   set partition_name = 'zope-' ~ custom_name -%}
@@ -288,6 +289,7 @@ software-type = zope
 {%   do zope_backend_path_dict.__setitem__(zope_family, backend_path) -%}
 {%   do ssl_authentication_dict.__setitem__(zope_family, zope_parameter_dict.get('ssl-authentication', False)) -%}
 {%   set current_zope_family_override_dict = zope_family_override_dict.get(zope_family, {}) -%}
+{%   do balancer_timeout_dict.__setitem__(zope_family, current_zope_family_override_dict.get('publisher-timeout', global_publisher_timeout)) -%}
 [{{ section_name }}]
 <= request-zope-base
 name = {{ partition_name }}
@@ -406,6 +408,7 @@ config-url = ${request-jupyter:connection-url}
     'zope-family-dict': zope_family_parameter_dict,
     'backend-path-dict': zope_backend_path_dict,
     'ssl-authentication-dict': ssl_authentication_dict,
+    'timeout-dict': balancer_timeout_dict,
     'apachedex-promise-threshold': monitor_dict.get('apachedex-promise-threshold', 70),
     'apachedex-configuration': monitor_dict.get(
       'apachedex-configuration',