diff --git a/Video/Dockerfile b/Video/Dockerfile index a6a426ee7..5a78e8a20 100644 --- a/Video/Dockerfile +++ b/Video/Dockerfile @@ -44,7 +44,7 @@ ENV DISPLAY_CONTAINER_NAME selenium ENV SE_SCREEN_WIDTH 1360 ENV SE_SCREEN_HEIGHT 1020 ENV SE_FRAME_RATE 15 -ENV SE_CODEC libx265 +ENV SE_CODEC libx264 ENV SE_PRESET "-preset ultrafast" ENV FILE_NAME video.mp4 diff --git a/charts/selenium-grid/README.md b/charts/selenium-grid/README.md index a1c6eff5b..1e2ee6bac 100644 --- a/charts/selenium-grid/README.md +++ b/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: 3600 + annotations: + nginx.ingress.kubernetes.io/proxy-connect-timeout: "7200" # This key will take 7200 instead of 3600 +``` + +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: @@ -111,6 +159,9 @@ This table contains the configuration parameters of the chart and their default | `ingress.enabled` | `true` | Enable or disable ingress resource | | `ingress.className` | `""` | Name of ingress class to select which controller will implement ingress resource | | `ingress.annotations` | `{}` | Custom annotations for ingress resource | +| `ingress.nginx.proxyTimeout` | `3600` | Value is used to set for NGINX ingress annotations related to proxy timeout | +| `ingress.nginx.proxyBuffer.size` | `512M` | Value is used to set for NGINX ingress annotations on size of the buffer proxy_buffer_size used for reading | +| `ingress.nginx.proxyBuffer.number` | `4` | Value is used to set for NGINX ingress annotations on number of the buffers in proxy_buffers used for reading | | `ingress.hostname` | `` | Default host for the ingress resource | | `ingress.path` | `/` | Default host path for the ingress resource | | `ingress.pathType` | `Prefix` | Default path type for the ingress resource | diff --git a/charts/selenium-grid/TESTING.md b/charts/selenium-grid/TESTING.md index e4fafe63e..7e94f85e4 100644 --- a/charts/selenium-grid/TESTING.md +++ b/charts/selenium-grid/TESTING.md @@ -16,6 +16,7 @@ All related testing to this helm chart will be documented in this file. | Ingress | Ingress is enabled without `hostname` | ✓ | Cluster | | | Ingress is enabled with `hostname` is set | ✗ | | | | Hub `sub-path` is set with Ingress `ImplementationSpecific` paths | ✓ | Cluster | +| | `ingress.nginx` configs for NGINX ingress controller annotations | ✓ | Template | | Distributed components | `isolateComponents` is enabled | ✓ | Cluster | | | `isolateComponents` is disabled | ✓ | Cluster | | Browser Nodes | Node `nameOverride` is set | ✓ | Cluster | diff --git a/charts/selenium-grid/templates/_helpers.tpl b/charts/selenium-grid/templates/_helpers.tpl index 1b0aa573c..9e44267d9 100644 --- a/charts/selenium-grid/templates/_helpers.tpl +++ b/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 */}} diff --git a/charts/selenium-grid/templates/ingress.yaml b/charts/selenium-grid/templates/ingress.yaml index f574c6c2f..215cfbace 100644 --- a/charts/selenium-grid/templates/ingress.yaml +++ b/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) }} diff --git a/charts/selenium-grid/values.yaml b/charts/selenium-grid/values.yaml index a32a9c96a..008d53b3b 100644 --- a/charts/selenium-grid/values.yaml +++ b/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: 3600 + proxyBuffer: + size: 512M + number: 4 # Custom annotations for ingress resource annotations: {} # Default host for the ingress resource diff --git a/tests/charts/templates/render/dummy.yaml b/tests/charts/templates/render/dummy.yaml index da60ed3a6..19ca7d6d9 100644 --- a/tests/charts/templates/render/dummy.yaml +++ b/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: 512M # Keep using sub-config default + number: # Disable sub-config + 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 diff --git a/tests/charts/templates/test.py b/tests/charts/templates/test.py index b004c2a8b..1e751f4e3 100644 --- a/tests/charts/templates/test.py +++ b/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-buffers-number') is None) + logger.info(f"Default annotation is added if no override value") + self.assertTrue(doc['metadata']['annotations']['nginx.ingress.kubernetes.io/client-body-buffer-size'] == '512M') + count += 1 + self.assertEqual(count, len(resources_name), "No ingress resources found") + if __name__ == '__main__': failed = False try: