feat(chart): Add default annotations for ingress nginx controller

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>

Author: VietND96

charts/selenium-grid/README.md

@@ -79,6 +79,54 @@ To uninstall:
 helm uninstall selenium-grid
 ```
 
+## Ingress Configuration
+
+By default, ingress is enabled without annotations set. If NGINX ingress controller is used, you need to set few annotations to override the default timeout values to avoid 504 errors (see #1808). Since in Selenium Grid the default of `SE_NODE_SESSION_TIMEOUT` and `SE_SESSION_REQUEST_TIMEOUT` is `300` seconds.
+
+In order to make user experience better, there are few annotations will be set by default if NGINX ingress controller is used. Mostly relates to timeouts and buffer sizes.
+
+If you are not using NGINX ingress controller, you can disable these default annotations by setting `ingress.nginx` to `nil` (aka null) via Helm CLI `--set ingress.nginx=null`) or via an override-values.yaml as below:
+
+```yaml
+ingress:
+  nginx:
+  # nginx: null (alternative way)
+```
+
+Similarly, if you want to disable a sub-config of `ingress.nginx`. For example: `--set ingress.nginx.proxyBuffer=null`)
+
+You are also able to combine using both default annotations and your own annotations in `ingress.annotations`. Duplicated keys will be merged strategy overwrite with your own annotations in `ingress.annotations` take precedence.
+
+```yaml
+ingress:
+  nginx:
+    proxyTimeout: 360
+  annotations:
+    nginx.ingress.kubernetes.io/proxy-connect-timeout: "3600" # This key will take 3600 instead of 360
+```
+
+List mapping of chart values and default annotation(s)
+
+```markdown
+# `ingress.nginx.proxyTimeout` pass value to annotation(s)
+nginx.ingress.kubernetes.io/proxy-connect-timeout
+nginx.ingress.kubernetes.io/proxy-send-timeout
+nginx.ingress.kubernetes.io/proxy-read-timeout
+nginx.ingress.kubernetes.io/proxy-next-upstream-timeout
+nginx.ingress.kubernetes.io/auth-keepalive-timeout
+
+# `ingress.nginx.proxyBuffer` pass value to to annotation(s)
+nginx.ingress.kubernetes.io/proxy-request-buffering: "on"
+nginx.ingress.kubernetes.io/proxy-buffering: "on"
+
+# `ingress.nginx.proxyBuffer.size` pass value to to annotation(s)
+nginx.ingress.kubernetes.io/proxy-buffer-size
+nginx.ingress.kubernetes.io/client-body-buffer-size
+
+# `ingress.nginx.proxyBuffer.number` pass value to annotation(s)
+nginx.ingress.kubernetes.io/proxy-buffers-number
+```
+
 ## Configuration
 
 For now, global configuration supported is:

charts/selenium-grid/templates/_helpers.tpl

@@ -80,6 +80,29 @@ Ingress fullname
 {{- default "selenium-ingress" .Values.ingress.nameOverride | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
+{{- define "seleniumGrid.ingress.nginx.annotations.default" -}}
+{{- with .Values.ingress.nginx }}
+  {{- with .proxyTimeout }}
+nginx.ingress.kubernetes.io/proxy-connect-timeout: {{ . | quote }}
+nginx.ingress.kubernetes.io/proxy-send-timeout: {{ . | quote }}
+nginx.ingress.kubernetes.io/proxy-read-timeout: {{ . | quote }}
+nginx.ingress.kubernetes.io/proxy-next-upstream-timeout: {{ . | quote }}
+nginx.ingress.kubernetes.io/auth-keepalive-timeout: {{ . | quote }}
+  {{- end }}
+  {{- with .proxyBuffer }}
+nginx.ingress.kubernetes.io/proxy-request-buffering: "on"
+nginx.ingress.kubernetes.io/proxy-buffering: "on"
+    {{- with .size }}
+nginx.ingress.kubernetes.io/proxy-buffer-size: {{ . | quote }}
+nginx.ingress.kubernetes.io/client-body-buffer-size: {{ . | quote }}
+    {{- end }}
+    {{- with .number }}
+nginx.ingress.kubernetes.io/proxy-buffers-number: {{ . | quote }}
+    {{- end }}
+  {{- end }}
+{{- end }}
+{{- end -}}
+
 {{/*
 Service Account fullname
 */}}

charts/selenium-grid/templates/ingress.yaml

@@ -20,9 +20,13 @@ metadata:
     {{- with .Values.customLabels }}
       {{- toYaml . | nindent 4 }}
     {{- end }}
-  {{- with .Values.ingress.annotations }}
+  {{- $ingressAnnotations := (include "seleniumGrid.ingress.nginx.annotations.default" . | toString | fromYaml ) }}
+  {{- with .Values.ingress.annotations -}}
+    {{- $ingressAnnotations = mergeOverwrite $ingressAnnotations . }}
+  {{- end }}
+  {{- if not (empty $ingressAnnotations) }}
   annotations:
-    {{- toYaml . | nindent 4 }}
+    {{- $ingressAnnotations | toYaml | nindent 4 }}
   {{- end }}
 spec:
   {{- if and .Values.ingress.className (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion) }}

charts/selenium-grid/values.yaml

@@ -36,6 +36,12 @@ ingress:
   enabled: true
   # Name of ingress class to select which controller will implement ingress resource
   className: ""
+  # Refer to list nginx annotations: https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#annotations
+  nginx:
+    proxyTimeout: 300
+    proxyBuffer:
+      size: 512m
+      number: 4
   # Custom annotations for ingress resource
   annotations: {}
   # Default host for the ingress resource

tests/charts/templates/render/dummy.yaml

@@ -11,6 +11,34 @@ global:
                   values:
                     - selenium
             topologyKey: "kubernetes.io/hostname"
+ingress:
+  nginx:
+    proxyTimeout: 360 # Set different proxy timout
+    proxyBuffer:
+      size: # Disable default
+      # number: 4 # Keep using sub-config default
+  annotations: # Add you own annotations
+    nginx.ingress.kubernetes.io/use-regex: "true" # Add new key
+    nginx.ingress.kubernetes.io/rewrite-target: /$2
+    nginx.ingress.kubernetes.io/app-root: &gridAppRoot "/selenium"
+    nginx.ingress.kubernetes.io/proxy-connect-timeout: "3600" # Override default key
+    nginx.ingress.kubernetes.io/proxy-send-timeout: "3600" # Override default key
+  hostname: ""
+  paths:
+    - path: /selenium(/|$)(.*)
+      pathType: ImplementationSpecific
+      backend:
+        service:
+          name: '{{ template "seleniumGrid.router.fullname" $ }}'
+          port:
+            number: 4444
+    - path: /(/?)(session/.*/se/vnc)
+      pathType: ImplementationSpecific
+      backend:
+        service:
+          name: '{{ template "seleniumGrid.router.fullname" $ }}'
+          port:
+            number: 4444
 
 isolateComponents: true
 

tests/charts/templates/test.py

@@ -20,12 +20,31 @@ def test_set_affinity(self):
         resources_name = ['selenium-chrome-node', 'selenium-distributor', 'selenium-edge-node', 'selenium-firefox-node',
                 'selenium-event-bus', 'selenium-router', 'selenium-session-map', 'selenium-session-queue']
         count = 0
+        logger.info(f"Assert affinity is set in global and nodes")
         for doc in LIST_OF_DOCUMENTS:
             if doc['metadata']['name'] in resources_name and doc['kind'] == 'Deployment':
+                logger.info(f"Assert affinity is set in resource {doc['metadata']['name']}")
                 self.assertTrue(doc['spec']['template']['spec']['affinity']['podAffinity']['requiredDuringSchedulingIgnoredDuringExecution'][0]['labelSelector']['matchExpressions'] is not None)
                 count += 1
         self.assertEqual(count, len(resources_name), "Not all resources have affinity set")
 
+    def test_ingress_nginx_annotations(self):
+        resources_name = ['selenium-ingress']
+        count = 0
+        for doc in LIST_OF_DOCUMENTS:
+            if doc['metadata']['name'] in resources_name and doc['kind'] == 'Ingress':
+                logger.info(f"Assert ingress ingress annotations")
+                logger.info(f"Config `ingress.nginx.proxyTimeout` is able to be set a different value")
+                self.assertTrue(doc['metadata']['annotations']['nginx.ingress.kubernetes.io/proxy-read-timeout'] == '360')
+                logger.info(f"Duplicated in `ingress.annotations` take precedence to overwrite the default value")
+                self.assertTrue(doc['metadata']['annotations']['nginx.ingress.kubernetes.io/proxy-connect-timeout'] == '3600')
+                logger.info(f"Default annotation is able to be disabled by setting it to null")
+                self.assertTrue(doc['metadata']['annotations'].get('nginx.ingress.kubernetes.io/proxy-buffer-size')  is None)
+                logger.info(f"Default annotation is added if no override value")
+                self.assertTrue(doc['metadata']['annotations']['nginx.ingress.kubernetes.io/proxy-buffers-number']  == '4')
+                count += 1
+        self.assertEqual(count, len(resources_name), "No ingress resources found")
+
 if __name__ == '__main__':
     failed = False
     try: