feat(chart): Simplify config probes, lifecycle hooks for Nodes

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
SeleniumHQ · Dec 27, 2023 · 5abf963 · 5abf963
1 parent 58ed283
commit 5abf963
Show file tree

Hide file tree

Showing 25 changed files with 601 additions and 165 deletions.
diff --git a/.github/workflows/helm-chart-test.yml b/.github/workflows/helm-chart-test.yml
@@ -67,10 +67,10 @@ jobs:
         with:
           name: ${{ matrix.test-strategy }}_${{ env.CHART_FILE_NAME }}
           path: ${{ env.CHART_PACKAGE_PATH }}
-      - name: Upload Helm chart template rendered
+      - name: Upload chart test artifacts
         if: always()
         uses: actions/upload-artifact@v4
         with:
-          name: ${{ matrix.test-strategy }}_chart_template_rendered.yaml
-          path: ./tests/tests/output_deployment.yaml
+          name: ${{ matrix.test-strategy }}-artifacts
+          path: ./tests/tests/
           if-no-files-found: ignore
diff --git a/Makefile b/Makefile
@@ -395,7 +395,7 @@ chart_test_edge:
 	VERSION=$(TAG_VERSION) NAMESPACE=$(NAMESPACE) ./tests/charts/make/chart_test.sh NodeEdge
 
 chart_test_parallel_autoscaling:
-	VERSION=$(TAG_VERSION) NAMESPACE=$(NAMESPACE) ./tests/charts/make/chart_test.sh ParallelAutoscaling
+	VERSION=$(TAG_VERSION) NAMESPACE=$(NAMESPACE) ./tests/charts/make/chart_test.sh JobAutoscaling
 
 .PHONY: \
 	all \

diff --git a/charts/selenium-grid/README.md b/charts/selenium-grid/README.md
diff --git a/charts/selenium-grid/TESTING.md b/charts/selenium-grid/TESTING.md
@@ -10,7 +10,7 @@ All related testing to this helm chart will be documented in this file.
 |                        | Basic Auth is enabled                                                | &cross;  |          |
 | Auto scaling           | Auto scaling with `enableWithExistingKEDA` is `true`                 | &check;  | Cluster  |
 |                        | Auto scaling with `scalingType` is `job`                             | &check;  | Cluster  |
-|                        | Auto scaling with `scalingType` is `deployment`                      | &cross;  |          |
+|                        | Auto scaling with `scalingType` is `deployment`                      | &check;  | Cluster  |
 |                        | Auto scaling with `autoscaling.scaledOptions.minReplicaCount` is `0` | &check;  | Cluster  |
 |                        | Parallel tests execution against node autoscaling                    | &check;  | Cluster  |
 | Ingress                | Ingress is enabled without `hostname`                                | &check;  | Cluster  |
@@ -27,6 +27,10 @@ All related testing to this helm chart will be documented in this file.
 |                        | Components are able to set `.affinity`                               | &check;  | Template |
 | Tracing                | Enable tracing via `SE_ENABLE_TRACING`                               | &check;  | Cluster  |
 |                        | Disable tracing via `SE_ENABLE_TRACING`                              | &check;  | Cluster  |
+| `Node` component       | `SE_NODE_PORT` can set a port different via `.port`                  | &check;  | Cluster  |
+|                        | Extra ports can be exposed on container via `.ports`                 | &check;  | Cluster  |
+|                        | Extra ports can be exposed on Service via `.service.ports`           | &check;  | Cluster  |
+|                        | Service type change to `NodePort`, specific NodePort can be set      | &check;  | Cluster  |
 
 ## Test Chart Template
 - By using `helm template` command, the chart template is tested without installing it to Kubernetes cluster.

diff --git a/charts/selenium-grid/templates/_helpers.tpl b/charts/selenium-grid/templates/_helpers.tpl
@@ -80,6 +80,13 @@ Ingress fullname
 {{- default "selenium-ingress" .Values.ingress.nameOverride | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
 
+{{/*
+Probe httpGet schema
+*/}}
+{{- define "seleniumGrid.probe.httpGet.schema" -}}
+{{- "HTTP" -}}
+{{- end -}}
+
 {{- define "seleniumGrid.ingress.nginx.annotations.default" -}}
 {{- with .Values.ingress.nginx }}
   {{- with .proxyTimeout }}
@@ -199,9 +206,12 @@ template:
         {{- $imageRegistry := default .Values.global.seleniumGrid.imageRegistry .node.imageRegistry }}
         image: {{ printf "%s/%s:%s" $imageRegistry .node.imageName $imageTag }}
         imagePullPolicy: {{ .node.imagePullPolicy }}
-      {{- with .node.extraEnvironmentVariables }}
-        env: {{- tpl (toYaml .) $ | nindent 10 }}
-      {{- end }}
+        env:
+          - name: SE_NODE_PORT
+            value: {{ .node.port | quote }}
+        {{- with .node.extraEnvironmentVariables }}
+          {{- tpl (toYaml .) $ | nindent 10 }}
+        {{- end }}
         envFrom:
           - configMapRef:
               name: {{ .Values.busConfigMap.name }}
@@ -212,16 +222,26 @@ template:
           {{- with .node.extraEnvFrom }}
             {{- tpl (toYaml .) $ | nindent 10 }}
           {{- end }}
-      {{- if gt (len .node.ports) 0 }}
         ports:
-        {{- range .node.ports }}
-          - containerPort: {{ . }}
+          - containerPort: {{ .node.port }}
             protocol: TCP
+      {{- if gt (len .node.ports) 0 }}
+        {{- $ports := .node.ports -}}
+        {{- if (regexMatch "[0-9]+$" (index $ports 0 | toString)) -}}
+          {{- range .node.ports }}
+          - containerPort: {{ . | int }}
+            protocol: TCP
+          {{- end }}
+        {{- else -}}
+          {{- tpl (toYaml .node.ports) $ | nindent 10 }}
         {{- end }}
       {{- end }}
         volumeMounts:
           - name: dshm
             mountPath: /dev/shm
+          - name: {{ .Values.nodeConfigMap.scriptVolumeMountName }}
+            mountPath: /opt/selenium/{{ .Values.nodeConfigMap.preStopScript }}
+            subPath: {{ .Values.nodeConfigMap.preStopScript }}
         {{- if .node.extraVolumeMounts }}
           {{- tpl (toYaml .node.extraVolumeMounts) $ | nindent 10 }}
         {{- end }}
@@ -231,12 +251,48 @@ template:
       {{- with .node.securityContext }}
         securityContext: {{- toYaml . | nindent 10 }}
       {{- end }}
-      {{- include "seleniumGrid.lifecycle" . | nindent 8 -}}
+      {{- include "seleniumGrid.node.lifecycle" . | nindent 8 -}}
+      {{- if .node.startupProbe.enabled }}
       {{- with .node.startupProbe }}
-        startupProbe: {{- toYaml . | nindent 10 }}
+        startupProbe:
+          httpGet:
+            scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .schema }}
+            path: {{ .path }}
+            port: {{ default ($.node.port) .port }}
+          initialDelaySeconds: {{ .initialDelaySeconds }}
+          periodSeconds: {{ .periodSeconds }}
+          timeoutSeconds: {{ .timeoutSeconds }}
+          successThreshold: {{ .successThreshold }}
+          failureThreshold: {{ .failureThreshold }}
       {{- end }}
+      {{- end }}
+      {{- if .node.readinessProbe.enabled }}
+      {{- with .node.readinessProbe }}
+        readinessProbe:
+          httpGet:
+            scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .schema }}
+            path: {{ .path }}
+            port: {{ default ($.node.port) .port }}
+          initialDelaySeconds: {{ .initialDelaySeconds }}
+          periodSeconds: {{ .periodSeconds }}
+          timeoutSeconds: {{ .timeoutSeconds }}
+          successThreshold: {{ .successThreshold }}
+          failureThreshold: {{ .failureThreshold }}
+      {{- end }}
+      {{- end }}
+      {{- if .node.livenessProbe.enabled }}
       {{- with .node.livenessProbe }}
-        livenessProbe: {{- toYaml . | nindent 10 }}
+        livenessProbe:
+          httpGet:
+            scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .schema }}
+            path: {{ .path }}
+            port: {{ default ($.node.port) .port }}
+          initialDelaySeconds: {{ .initialDelaySeconds }}
+          periodSeconds: {{ .periodSeconds }}
+          timeoutSeconds: {{ .timeoutSeconds }}
+          successThreshold: {{ .successThreshold }}
+          failureThreshold: {{ .failureThreshold }}
+      {{- end }}
       {{- end }}
     {{- if .node.sidecars }}
       {{- toYaml .node.sidecars | nindent 6 }}
@@ -325,6 +381,10 @@ template:
   {{- end }}
     terminationGracePeriodSeconds: {{ .node.terminationGracePeriodSeconds }}
     volumes:
+      - name: {{ .Values.nodeConfigMap.scriptVolumeMountName }}
+        configMap:
+          name: {{ .Values.nodeConfigMap.name }}
+          defaultMode: {{ .Values.nodeConfigMap.defaultMode }}
       - name: dshm
         emptyDir:
           medium: Memory
@@ -425,19 +485,55 @@ Graphql unsafeSsl of the hub or the router
 {{- end -}}
 
 {{/*
-Get the lifecycle of the pod. When KEDA is activated and the lifecycle is used for a pod of a
-deployment preStop hook to deregister from the selenium hub.
+Define preStop hook for the node pod. Node preStop script is stored in a ConfigMap and mounted as a volume.
 */}}
-{{- define "seleniumGrid.lifecycle" }}
-{{ $lifecycle := tpl (toYaml (default (dict) .node.lifecycle)) $ }}
-{{- if and (eq .Values.autoscaling.scalingType "deployment") (eq (include "seleniumGrid.useKEDA" .) "true") -}}
-{{ $lifecycle = merge ($lifecycle | fromYaml ) .Values.autoscaling.deregisterLifecycle | toYaml }}
+{{- define "seleniumGrid.node.deregisterLifecycle" -}}
+preStop:
+  exec:
+    command: ["bash", "-c", "/opt/selenium/{{ .Values.nodeConfigMap.preStopScript }}"]
+{{- end -}}
+
+{{/*
+Get the lifecycle of the pod is used for a Node to deregister from the Hub/Router.
+1. IF KEDA is activated, scalingType is "deployment", and individual node deregisterLifecycle is not set, use autoscaling.deregisterLifecycle
+2. ELSE (KEDA is not activated and node deregisterLifecycle is set), use .deregisterLifecycle in individual node
+3. IF individual node with .lifecycle is set, it takes highest precedence to override the preStop in above use cases
+*/}}
+{{- define "seleniumGrid.node.lifecycle" }}
+{{- $defaultDeregisterLifecycle := tpl (include "seleniumGrid.node.deregisterLifecycle" .) $ -}}
+{{- $lifecycle := toYaml (dict) -}}
+{{- if and (and (eq .Values.autoscaling.scalingType "deployment") (eq (include "seleniumGrid.useKEDA" .) "true")) (empty .node.deregisterLifecycle) -}}
+  {{- $lifecycle = merge ($lifecycle | fromYaml) (tpl (toYaml (default ($defaultDeregisterLifecycle | fromYaml) .Values.autoscaling.deregisterLifecycle)) $ | fromYaml) | toYaml -}}
+{{- else -}}
+  {{- if eq (.node.deregisterLifecycle | toString | lower) "false" -}}
+    {{- $lifecycle = toYaml (dict) -}}
+  {{- else -}}
+    {{- $lifecycle = (tpl (toYaml (default ($defaultDeregisterLifecycle | fromYaml) .node.deregisterLifecycle) ) $ | fromYaml) | toYaml -}}
+  {{- end -}}
+{{- end -}}
+{{- if not (empty .node.lifecycle) -}}
+  {{- $lifecycle = mergeOverwrite ($lifecycle | fromYaml) (tpl (toYaml .node.lifecycle) $ | fromYaml) | toYaml -}}
 {{- end -}}
 {{ if and $lifecycle (ne $lifecycle "{}") -}}
 lifecycle: {{ $lifecycle | nindent 2 }}
 {{- end -}}
 {{- end -}}
 
+{{/*
+Define terminationGracePeriodSeconds of the node pod.
+1. IF KEDA is activated, scalingType is "deployment", use autoscaling.terminationGracePeriodSeconds
+2. IF node.terminationGracePeriodSeconds is greater than autoscaling.terminationGracePeriodSeconds, use node.terminationGracePeriodSeconds
+*/}}
+{{- define "seleniumGrid.node.terminationGracePeriodSeconds" -}}
+{{- $autoscalingPeriod := default 0 .Values.autoscaling.terminationGracePeriodSeconds -}}
+{{- $nodePeriod := default 0 .node.terminationGracePeriodSeconds -}}
+{{- $period := $nodePeriod -}}
+{{- if and (eq .Values.autoscaling.scalingType "deployment") (eq (include "seleniumGrid.useKEDA" .) "true") -}}
+  {{- $period = ternary $nodePeriod $autoscalingPeriod (gt $nodePeriod $autoscalingPeriod) -}}
+{{- end -}}
+{{- $period -}}
+{{- end -}}
+
 {{/*
 Default specs of VolumeMounts and Volumes for video recorder
 */}}

diff --git a/charts/selenium-grid/templates/chrome-node-service.yaml b/charts/selenium-grid/templates/chrome-node-service.yaml
@@ -22,8 +22,11 @@ spec:
   ports:
     - name: tcp-chrome
       protocol: TCP
-      port: {{ .Values.chromeNode.seleniumServicePort }}
-      targetPort: {{ .Values.chromeNode.seleniumPort }}
+      port: {{ .Values.chromeNode.port }}
+      targetPort: {{ .Values.chromeNode.port }}
+      {{- if and (eq $.Values.chromeNode.service.type "NodePort") .Values.chromeNode.nodePort }}
+      nodePort: {{ .Values.chromeNode.nodePort }}
+      {{- end }}
   {{- with .Values.chromeNode.service.ports }}
     {{- range . }}
     - name: {{ .name }}

diff --git a/charts/selenium-grid/templates/edge-node-service.yaml b/charts/selenium-grid/templates/edge-node-service.yaml
@@ -22,8 +22,11 @@ spec:
   ports:
     - name: tcp-edge
       protocol: TCP
-      port: {{ .Values.edgeNode.seleniumServicePort }}
-      targetPort: {{ .Values.edgeNode.seleniumPort }}
+      port: {{ .Values.edgeNode.port }}
+      targetPort: {{ .Values.edgeNode.port }}
+      {{- if and (eq $.Values.edgeNode.service.type "NodePort") .Values.edgeNode.nodePort }}
+      nodePort: {{ .Values.edgeNode.nodePort }}
+      {{- end }}
   {{- with .Values.edgeNode.service.ports }}
     {{- range . }}
     - name: {{ .name }}

diff --git a/charts/selenium-grid/templates/event-bus-configmap.yaml b/charts/selenium-grid/templates/event-bus-configmap.yaml
@@ -1,4 +1,4 @@
-{{- $eventBusHost := ternary (include "seleniumGrid.eventBus.fullname" .) (include "seleniumGrid.hub.fullname" .) .Values.isolateComponents -}}
+{{- $eventBusHost := printf "%s.%s" (ternary (include "seleniumGrid.eventBus.fullname" .) (include "seleniumGrid.hub.fullname" .) .Values.isolateComponents) (.Release.Namespace) -}}
 {{- $eventBusPublishPort := ternary .Values.components.eventBus.publishPort .Values.hub.publishPort .Values.isolateComponents -}}
 {{- $eventBusSubscribePort := ternary .Values.components.eventBus.subscribePort .Values.hub.subscribePort .Values.isolateComponents -}}
 apiVersion: v1

diff --git a/charts/selenium-grid/templates/firefox-node-service.yaml b/charts/selenium-grid/templates/firefox-node-service.yaml
@@ -22,8 +22,11 @@ spec:
   ports:
     - name: tcp-firefox
       protocol: TCP
-      port: {{ .Values.firefoxNode.seleniumServicePort }}
-      targetPort: {{ .Values.firefoxNode.seleniumPort }}
+      port: {{ .Values.firefoxNode.port }}
+      targetPort: {{ .Values.firefoxNode.port }}
+      {{- if and (eq $.Values.firefoxNode.service.type "NodePort") .Values.firefoxNode.nodePort }}
+      nodePort: {{ .Values.firefoxNode.nodePort }}
+      {{- end }}
   {{- with .Values.firefoxNode.service.ports }}
     {{- range . }}
     - name: {{ .name }}

diff --git a/charts/selenium-grid/templates/hub-deployment.yaml b/charts/selenium-grid/templates/hub-deployment.yaml
@@ -45,8 +45,9 @@ spec:
         {{- if .Values.hub.livenessProbe.enabled }}
           livenessProbe:
             httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.hub.livenessProbe.schema }}
               path: {{ .Values.hub.livenessProbe.path }}
-              port: {{ .Values.hub.port }}
+              port: {{ default (.Values.hub.port) .Values.hub.livenessProbe.port }}
             initialDelaySeconds: {{ .Values.hub.livenessProbe.initialDelaySeconds }}
             periodSeconds: {{ .Values.hub.livenessProbe.periodSeconds }}
             timeoutSeconds: {{ .Values.hub.livenessProbe.timeoutSeconds }}
@@ -56,13 +57,26 @@ spec:
         {{- if .Values.hub.readinessProbe.enabled }}
           readinessProbe:
             httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.hub.readinessProbe.schema }}
               path: {{ .Values.hub.readinessProbe.path }}
-              port: {{ .Values.hub.port }}
+              port: {{ default (.Values.hub.port) .Values.hub.readinessProbe.port }}
             initialDelaySeconds: {{ .Values.hub.readinessProbe.initialDelaySeconds }}
             periodSeconds: {{ .Values.hub.readinessProbe.periodSeconds }}
             timeoutSeconds: {{ .Values.hub.readinessProbe.timeoutSeconds }}
             successThreshold: {{ .Values.hub.readinessProbe.successThreshold }}
             failureThreshold: {{ .Values.hub.readinessProbe.failureThreshold }}
+        {{- end }}
+        {{- if .Values.hub.startupProbe.enabled }}
+          startupProbe:
+            httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.hub.startupProbe.schema }}
+              path: {{ .Values.hub.startupProbe.path }}
+              port: {{ default (.Values.hub.port) .Values.hub.startupProbe.port }}
+            initialDelaySeconds: {{ .Values.hub.startupProbe.initialDelaySeconds }}
+            periodSeconds: {{ .Values.hub.startupProbe.periodSeconds }}
+            timeoutSeconds: {{ .Values.hub.startupProbe.timeoutSeconds }}
+            successThreshold: {{ .Values.hub.startupProbe.successThreshold }}
+            failureThreshold: {{ .Values.hub.startupProbe.failureThreshold }}
         {{- end }}
           env:
             {{- with .Values.hub.subPath }}

diff --git a/charts/selenium-grid/templates/node-configmap.yaml b/charts/selenium-grid/templates/node-configmap.yaml
@@ -14,3 +14,11 @@ metadata:
 data:
   SE_DRAIN_AFTER_SESSION_COUNT: '{{- and (eq (include "seleniumGrid.useKEDA" .) "true") (eq .Values.autoscaling.scalingType "job") | ternary "1" "0" -}}'
   SE_NODE_GRID_URL: '{{ include "seleniumGrid.url" .}}'
+  {{ .Values.nodeConfigMap.preStopScript }}: |
+    #!/bin/bash
+    if curl -sf 127.0.0.1:${SE_NODE_PORT}/status; then
+        curl -X POST 127.0.0.1:${SE_NODE_PORT}/se/grid/node/drain --header 'X-REGISTRATION-SECRET;'
+        while curl -sf 127.0.0.1:${SE_NODE_PORT}/status; do sleep 1; done
+    else
+        echo "Node is already drained. Shutting down gracefully!"
+    fi
diff --git a/charts/selenium-grid/templates/router-deployment.yaml b/charts/selenium-grid/templates/router-deployment.yaml
@@ -70,8 +70,9 @@ spec:
         {{- if .Values.components.router.livenessProbe.enabled }}
           livenessProbe:
             httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.components.router.livenessProbe.schema }}
               path: {{ .Values.components.router.livenessProbe.path }}
-              port: {{ .Values.components.router.port }}
+              port: {{ default (.Values.components.router.port) .Values.components.router.livenessProbe.port }}
             initialDelaySeconds: {{ .Values.components.router.livenessProbe.initialDelaySeconds }}
             periodSeconds: {{ .Values.components.router.livenessProbe.periodSeconds }}
             timeoutSeconds: {{ .Values.components.router.livenessProbe.timeoutSeconds }}
@@ -81,14 +82,27 @@ spec:
         {{- if .Values.components.router.readinessProbe.enabled }}
           readinessProbe:
             httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.components.router.readinessProbe.schema }}
               path: {{ .Values.components.router.readinessProbe.path }}
-              port: {{ .Values.components.router.port }}
+              port: {{ default (.Values.components.router.port) .Values.components.router.readinessProbe.port }}
             initialDelaySeconds: {{ .Values.components.router.readinessProbe.initialDelaySeconds }}
             periodSeconds: {{ .Values.components.router.readinessProbe.periodSeconds }}
             timeoutSeconds: {{ .Values.components.router.readinessProbe.timeoutSeconds }}
             successThreshold: {{ .Values.components.router.readinessProbe.successThreshold }}
             failureThreshold: {{ .Values.components.router.readinessProbe.failureThreshold }}
         {{- end }}
+        {{- if .Values.components.router.startupProbe.enabled }}
+          startupProbe:
+            httpGet:
+              scheme: {{ default (include "seleniumGrid.probe.httpGet.schema" .) .Values.components.router.startupProbe.schema }}
+              path: {{ .Values.components.router.startupProbe.path }}
+              port: {{ default (.Values.components.router.port) .Values.components.router.startupProbe.port }}
+            initialDelaySeconds: {{ .Values.components.router.startupProbe.initialDelaySeconds }}
+            periodSeconds: {{ .Values.components.router.startupProbe.periodSeconds }}
+            timeoutSeconds: {{ .Values.components.router.startupProbe.timeoutSeconds }}
+            successThreshold: {{ .Values.components.router.startupProbe.successThreshold }}
+            failureThreshold: {{ .Values.components.router.startupProbe.failureThreshold }}
+        {{- end }}
         {{- with .Values.components.router.resources }}
           resources: {{- toYaml . | nindent 12 }}
         {{- end }}