Add option to enable worker graceful shutdown

trinodb · Oct 1, 2024 · 84d69aa · 84d69aa
1 parent 36a4833
commit 84d69aa
Show file tree

Hide file tree

Showing 11 changed files with 242 additions and 13 deletions.
diff --git a/charts/trino/README.md b/charts/trino/README.md
@@ -497,9 +497,11 @@ Fast distributed SQL query engine for big data analytics that helps you explore
 
   Allows mounting additional Trino configuration files from Kubernetes secrets on the coordinator node.
   Example:
+  ```yaml
    - name: sample-secret
      secretName: sample-secret
      path: /secrets/sample.json
+  ```
 * `worker.jvm.maxHeapSize` - string, default: `"8G"`
 * `worker.jvm.gcMethod.type` - string, default: `"UseG1GC"`
 * `worker.jvm.gcMethod.g1.heapRegionSize` - string, default: `"32M"`
@@ -553,12 +555,21 @@ Fast distributed SQL query engine for big data analytics that helps you explore
   ```
 * `worker.lifecycle` - object, default: `{}`  
 
-  To enable [graceful shutdown](https://trino.io/docs/current/admin/graceful-shutdown.html), define a lifecycle preStop like bellow, Set the `terminationGracePeriodSeconds` to a value greater than or equal to the configured `shutdown.grace-period`. Configure `shutdown.grace-period` in `additionalConfigProperties` as `shutdown.grace-period=2m` (default is 2 minutes). Also configure `accessControl` because the `default` system access control does not allow graceful shutdowns.
+  Worker container [lifecycle events](https://kubernetes.io/docs/tasks/configure-pod-container/attach-handler-lifecycle-event/)  Setting `worker.lifecycle` conflicts with `worker.gracefulShutdown`.
   Example:
   ```yaml
    preStop:
      exec:
-       command: ["/bin/sh", "-c", "curl -v -X PUT -d '\"SHUTTING_DOWN\"' -H \"Content-type: application/json\" http://localhost:8081/v1/info/state"]
+       command: ["/bin/sh", "-c", "sleep 120"]
+  ```
+* `worker.gracefulShutdown` - object, default: `{"enabled":false,"gracePeriodSeconds":120}`  
+
+  Configure [graceful shutdown](https://trino.io/docs/current/admin/graceful-shutdown.html)  Enabling this feature will:  1) Add a `preStop` lifecycle event to all worker Pods;  2) Set the `shutdown.grace-period` configuration property to `gracePeriod`;  3) Configure the workers' `accessControl` since the `default` system access control [does not allow graceful    shutdowns](https://trino.io/docs/current/admin/graceful-shutdown.html). The user must set the `terminationGracePeriodSeconds` to a value of at least two times the configured `gracePeriodSeconds`. The worker that receives the graceful shutdown request [will sleep for `gracePeriod` twice](https://trino.io/docs/current/admin/graceful-shutdown.html#shutdown-behavior).  Enabling `worker.gracefulShutdown` conflicts with `worker.lifecycle`. If you need to provide a custom `worker.lifecycle` configuration, and you want to enable `worker.gracefulShutdown`, you have to do so manually.
+  Example:
+  ```yaml
+   gracefulShutdown:
+     enabled: true
+     gracePeriodSeconds: 120
   ```
 * `worker.terminationGracePeriodSeconds` - int, default: `30`
 * `worker.nodeSelector` - object, default: `{}`

diff --git a/...o/templates/configmap-access-control.yaml → ...configmap-access-control-coordinator.yaml b/...o/templates/configmap-access-control.yaml → ...configmap-access-control-coordinator.yaml
diff --git a/charts/trino/templates/configmap-access-control-worker.yaml b/charts/trino/templates/configmap-access-control-worker.yaml
@@ -0,0 +1,22 @@
+{{- if .Values.worker.gracefulShutdown.enabled }}
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: {{ template "trino.fullname" . }}-access-control-volume-worker
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "trino.labels" . | nindent 4 }}
+    app.kubernetes.io/component: worker
+data:
+  graceful-shutdown-rules.json: >-
+    {
+      "system_information": [
+        {
+          "allow": [
+            "write"
+          ],
+          "user": "admin"
+        }
+      ]
+    }
+{{- end }}
diff --git a/charts/trino/templates/configmap-coordinator.yaml b/charts/trino/templates/configmap-coordinator.yaml
@@ -76,6 +76,9 @@ data:
     jmx.rmiregistry.port={{ .Values.jmx.registryPort }}
     jmx.rmiserver.port={{ .Values.jmx.serverPort }}
     {{- end }}
+    {{- if .Values.worker.gracefulShutdown.enabled }}
+    shutdown.grace-period={{- .Values.worker.gracefulShutdown.gracePeriodSeconds -}}s
+    {{- end }}
     {{- if .Values.server.coordinatorExtraConfig }}
     {{- .Values.server.coordinatorExtraConfig | nindent 4 }}
     {{- end }}

diff --git a/charts/trino/templates/configmap-worker.yaml b/charts/trino/templates/configmap-worker.yaml
@@ -57,10 +57,19 @@ data:
     {{- range $configValue := .Values.additionalConfigProperties }}
     {{ $configValue }}
     {{- end }}
+    {{- if .Values.worker.gracefulShutdown.enabled }}
+    shutdown.grace-period={{- .Values.worker.gracefulShutdown.gracePeriodSeconds -}}s
+    {{- end }}
     {{- if .Values.server.workerExtraConfig }}
     {{- .Values.server.workerExtraConfig | nindent 4 }}
     {{- end }}
 
+{{- if .Values.worker.gracefulShutdown.enabled }}
+  access-control.properties: |
+    access-control.name=file
+    security.config-file={{ .Values.server.config.path }}/access-control/graceful-shutdown-rules.json
+{{- end }}
+
 {{- if .Values.server.exchangeManager }}
   exchange-manager.properties: |
     exchange-manager.name={{ .Values.server.exchangeManager.name }}

diff --git a/charts/trino/templates/deployment-coordinator.yaml b/charts/trino/templates/deployment-coordinator.yaml
@@ -18,7 +18,7 @@ spec:
     metadata:
       annotations:
         {{- if and (eq .Values.accessControl.type "configmap") (not .Values.accessControl.refreshPeriod) }}
-        checksum/access-control-config: {{ include (print $.Template.BasePath "/configmap-access-control.yaml") . | sha256sum }}
+        checksum/access-control-config: {{ include (print $.Template.BasePath "/configmap-access-control-coordinator.yaml") . | sha256sum }}
         {{- end }}
         {{- if or .Values.catalogs .Values.additionalCatalogs }}
         checksum/catalog-config: {{ include (print $.Template.BasePath "/configmap-catalog.yaml") . | sha256sum }}

diff --git a/charts/trino/templates/deployment-worker.yaml b/charts/trino/templates/deployment-worker.yaml
@@ -25,6 +25,9 @@ spec:
         checksum/catalog-config: {{ include (print $.Template.BasePath "/configmap-catalog.yaml") . | sha256sum }}
         {{- end }}
         checksum/worker-config: {{ include (print $.Template.BasePath "/configmap-worker.yaml") . | sha256sum }}
+      {{- if .Values.worker.gracefulShutdown.enabled }}
+        checksum/access-control-config: {{ include (print $.Template.BasePath "/configmap-access-control-worker.yaml") . | sha256sum }}
+      {{- end }}
       {{- if .Values.worker.annotations }}
       {{- tpl (toYaml .Values.worker.annotations) . | nindent 8 }}
       {{- end }}
@@ -55,6 +58,11 @@ spec:
         - name: schemas-volume
           configMap:
             name: {{ template "trino.fullname" . }}-schemas-volume-worker
+        {{- if .Values.worker.gracefulShutdown.enabled }}
+        - name: access-control-volume
+          configMap:
+            name: {{ template "trino.fullname" . }}-access-control-volume-worker
+        {{- end }}
         {{- range .Values.configMounts }}
         - name: {{ .name }}
           configMap:
@@ -84,7 +92,11 @@ spec:
       {{- end }}
       imagePullSecrets:
         {{- toYaml .Values.imagePullSecrets | nindent 8 }}
+      {{- if and .Values.worker.gracefulShutdown.enabled (gt (mulf 2.0 .Values.worker.gracefulShutdown.gracePeriodSeconds) .Values.worker.terminationGracePeriodSeconds) }}
+      {{- fail "The user must set the `worker.terminationGracePeriodSeconds` to a value of at least two times the configured `gracePeriodSeconds`." }}
+      {{- else }}
       terminationGracePeriodSeconds: {{ .Values.worker.terminationGracePeriodSeconds }}
+      {{- end }}
       containers:
         - name: {{ .Chart.Name }}-worker
           image: {{ include "trino.image" . }}
@@ -104,6 +116,10 @@ spec:
             {{- end }}
             - mountPath: {{ .Values.kafka.mountPath }}
               name: schemas-volume
+            {{- if .Values.worker.gracefulShutdown.enabled }}
+            - mountPath: {{ .Values.server.config.path }}/access-control
+              name: access-control-volume
+            {{- end }}
             {{- range .Values.configMounts }}
             - name: {{ .name }}
               mountPath: {{ .path }}
@@ -150,7 +166,24 @@ spec:
             failureThreshold: {{ .Values.worker.readinessProbe.failureThreshold | default 6 }}
             successThreshold: {{ .Values.worker.readinessProbe.successThreshold | default 1 }}
           lifecycle:
+            {{- if .Values.worker.lifecycle }}
+            {{- if .Values.worker.gracefulShutdown.enabled }}
+            {{- fail "The `worker.lifecycle` configuration conflicts with `worker.gracefulShutdown`. Either disable `worker.gracefulShutdown` (and apply the related configurations manually), or remove `worker.lifecycle`." }}
+            {{- end }}
             {{- toYaml .Values.worker.lifecycle | nindent 12 }}
+            {{- else if .Values.worker.gracefulShutdown.enabled }}
+            preStop:
+              exec:
+                command:
+                  - /bin/sh
+                  - -c
+                  - >-
+                    curl -v -X PUT
+                    -d '"SHUTTING_DOWN"'
+                    -H 'Content-type: application/json'
+                    -H 'X-Trino-User: admin'
+                    http://localhost:{{- .Values.service.port -}}/v1/info/state
+            {{- end }}
           resources:
             {{- toYaml .Values.worker.resources | nindent 12 }}
       {{- if .Values.sidecarContainers.worker }}

diff --git a/charts/trino/templates/tests/test-graceful-shutdown.yaml b/charts/trino/templates/tests/test-graceful-shutdown.yaml
@@ -0,0 +1,120 @@
+{{- if .Values.worker.gracefulShutdown.enabled }}
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: {{ include "trino.fullname" . }}-pod-manager
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "trino.labels" . | nindent 4 }}
+    app.kubernetes.io/component: test
+    test: graceful-shutdown
+  annotations:
+    "helm.sh/hook": test
+    "helm.sh/hook-weight": "0"
+    "helm.sh/hook-delete-policy": hook-succeeded
+rules:
+  - apiGroups: [ "" ]
+    resources: [ "pods" ]
+    verbs: [ "get", "list", "delete" ]
+  - apiGroups: [ "" ]
+    resources: [ "pods/log" ]
+    verbs: [ "get" ]
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ include "trino.fullname" . }}-pod-manager-sa
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "trino.labels" . | nindent 4 }}
+    app.kubernetes.io/component: test
+    test: graceful-shutdown
+  annotations:
+    "helm.sh/hook": test
+    "helm.sh/hook-weight": "0"
+    "helm.sh/hook-delete-policy": hook-succeeded
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: {{ include "trino.fullname" . }}-pod-manager-binding
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "trino.labels" . | nindent 4 }}
+    app.kubernetes.io/component: test
+    test: graceful-shutdown
+  annotations:
+    "helm.sh/hook": test
+    "helm.sh/hook-weight": "1"
+    "helm.sh/hook-delete-policy": hook-succeeded
+subjects:
+  - kind: ServiceAccount
+    name: {{ include "trino.fullname" . }}-pod-manager-sa
+    namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: Role
+  name: {{ include "trino.fullname" . }}-pod-manager
+  apiGroup: rbac.authorization.k8s.io
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: {{ include "trino.fullname" . }}-test-graceful-shutdown
+  labels:
+    {{- include "trino.labels" . | nindent 4 }}
+    app.kubernetes.io/component: test
+    test: graceful-shutdown
+  annotations:
+    "helm.sh/hook": test
+    "helm.sh/hook-weight": "2"
+    "helm.sh/hook-delete-policy": hook-succeeded
+spec:
+  serviceAccountName: {{ include "trino.fullname" . }}-pod-manager-sa
+  initContainers:
+    - name: get-worker-pod
+      image: bitnami/kubectl:latest
+      command: [ "sh", "-c" ]
+      args:
+        - >-
+          kubectl get pods
+          --selector="app.kubernetes.io/name={{ include "trino.name" . }},app.kubernetes.io/instance={{ .Release.Name }},app.kubernetes.io/component=worker"
+          --output=jsonpath="{.items[0].metadata.name}"
+          --namespace={{ .Release.Namespace }}
+          > /pods/worker-pod.txt
+      volumeMounts:
+        - mountPath: /pods
+          name: worker-pod
+  containers:
+    - name: check-logs
+      image: bitnami/kubectl:latest
+      command: [ "sh", "-c" ]
+      args:
+        - >-
+          WORKER_POD=$(cat /pods/worker-pod.txt) &&
+          kubectl logs ${WORKER_POD}
+          --follow
+          --container=trino-worker
+          --namespace={{ .Release.Namespace }}
+          | grep --max-count=1 "Shutdown requested"
+      volumeMounts:
+        - mountPath: /pods
+          name: worker-pod
+    - name: trigger-graceful-shutdown
+      image: bitnami/kubectl:latest
+      command: [ "sh", "-c" ]
+      args:
+        - >-
+          sleep 5 &&
+          WORKER_POD=$(cat /pods/worker-pod.txt) &&
+          kubectl delete pod
+          ${WORKER_POD}
+          --namespace={{ .Release.Namespace }}
+      volumeMounts:
+        - mountPath: /pods
+          name: worker-pod
+  restartPolicy: Never
+  volumes:
+    - name: worker-pod
+      emptyDir: {}
+
+{{- end }}
diff --git a/charts/trino/values.yaml b/charts/trino/values.yaml
@@ -582,9 +582,11 @@ coordinator:
   # files from Kubernetes secrets on the coordinator node.
   # @raw
   # Example:
+  # ```yaml
   #  - name: sample-secret
   #    secretName: sample-secret
   #    path: /secrets/sample.json
+  # ```
 
 worker:
   jvm:
@@ -657,20 +659,42 @@ worker:
   # ```
 
   lifecycle: {}
-  # worker.lifecycle -- To enable [graceful
-  # shutdown](https://trino.io/docs/current/admin/graceful-shutdown.html),
-  # define a lifecycle preStop like bellow, Set the
-  # `terminationGracePeriodSeconds` to a value greater than or equal to the
-  # configured `shutdown.grace-period`. Configure `shutdown.grace-period` in
-  # `additionalConfigProperties` as `shutdown.grace-period=2m` (default is 2
-  # minutes). Also configure `accessControl` because the `default` system
-  # access control does not allow graceful shutdowns.
+  # worker.lifecycle -- Worker container [lifecycle
+  # events](https://kubernetes.io/docs/tasks/configure-pod-container/attach-handler-lifecycle-event/)
+  #
+  # Setting `worker.lifecycle` conflicts with `worker.gracefulShutdown`.
+  #
   # @raw
   # Example:
   # ```yaml
   #  preStop:
   #    exec:
-  #      command: ["/bin/sh", "-c", "curl -v -X PUT -d '\"SHUTTING_DOWN\"' -H \"Content-type: application/json\" http://localhost:8081/v1/info/state"]
+  #      command: ["/bin/sh", "-c", "sleep 120"]
+  # ```
+
+  gracefulShutdown:
+    enabled: false
+    gracePeriodSeconds: 120
+  # worker.gracefulShutdown -- Configure [graceful
+  # shutdown](https://trino.io/docs/current/admin/graceful-shutdown.html)
+  #
+  # Enabling this feature will:
+  #  1) Add a `preStop` lifecycle event to all worker Pods;
+  #  2) Set the `shutdown.grace-period` configuration property to `gracePeriod`;
+  #  3) Configure the workers' `accessControl` since the `default` system access control [does not allow graceful
+  #    shutdowns](https://trino.io/docs/current/admin/graceful-shutdown.html).
+  # The user must set the `terminationGracePeriodSeconds` to a value of at least two times the configured `gracePeriodSeconds`.
+  # The worker that receives the graceful shutdown request [will sleep for `gracePeriod` twice](https://trino.io/docs/current/admin/graceful-shutdown.html#shutdown-behavior).
+  #
+  # Enabling `worker.gracefulShutdown` conflicts with `worker.lifecycle`. If you need to provide a custom
+  # `worker.lifecycle` configuration, and you want to enable `worker.gracefulShutdown`, you have to do so manually.
+  #
+  # @raw
+  # Example:
+  # ```yaml
+  #  gracefulShutdown:
+  #    enabled: true
+  #    gracePeriodSeconds: 120
   # ```
 
   terminationGracePeriodSeconds: 30

diff --git a/test-graceful-shutdown-values.yaml b/test-graceful-shutdown-values.yaml
@@ -0,0 +1,6 @@
+worker:
+  gracefulShutdown:
+    enabled: true
+    gracePeriodSeconds: 60
+
+  terminationGracePeriodSeconds: 120
diff --git a/test.sh b/test.sh
@@ -9,6 +9,7 @@ declare -A testCases=(
     [overrides]="--set coordinatorNameOverride=coordinator-overridden,workerNameOverride=worker-overridden,nameOverride=overridden"
     [access_control_properties_values]="--values test-access-control-properties-values.yaml"
     [exchange_manager_values]="--values test-exchange-manager-values.yaml"
+    [graceful_shutdown]="--values test-graceful-shutdown-values.yaml"
 )
 
 function join_by {
@@ -23,7 +24,7 @@ NAMESPACE=trino-$(LC_ALL=C tr -dc 'a-z0-9' </dev/urandom | head -c 6 || true)
 HELM_EXTRA_SET_ARGS=
 CT_ARGS=(--charts=charts/trino --skip-clean-up --helm-extra-args="--timeout 2m")
 CLEANUP_NAMESPACE=true
-TEST_NAMES=(default single_node complete_values access_control_properties_values exchange_manager_values)
+TEST_NAMES=(default single_node complete_values access_control_properties_values exchange_manager_values graceful_shutdown)
 
 usage() {
     cat <<EOF 1>&2