From 35d729c20ef67d90eed5720babb2a352055fc5a3 Mon Sep 17 00:00:00 2001 From: Sebastian Daberdaku Date: Thu, 26 Sep 2024 12:47:21 +0200 Subject: [PATCH] Add option to enable worker graceful shutdown --- charts/trino/README.md | 15 ++- ...configmap-access-control-coordinator.yaml} | 0 .../configmap-access-control-worker.yaml | 22 ++++ .../templates/configmap-coordinator.yaml | 3 + charts/trino/templates/configmap-worker.yaml | 9 ++ .../templates/deployment-coordinator.yaml | 2 +- charts/trino/templates/deployment-worker.yaml | 33 +++++ .../tests/test-graceful-shutdown.yaml | 120 ++++++++++++++++++ charts/trino/values.yaml | 42 ++++-- test-graceful-shutdown-values.yaml | 6 + test.sh | 3 +- 11 files changed, 242 insertions(+), 13 deletions(-) rename charts/trino/templates/{configmap-access-control.yaml => configmap-access-control-coordinator.yaml} (100%) create mode 100644 charts/trino/templates/configmap-access-control-worker.yaml create mode 100644 charts/trino/templates/tests/test-graceful-shutdown.yaml create mode 100644 test-graceful-shutdown-values.yaml diff --git a/charts/trino/README.md b/charts/trino/README.md index 2231da3b..a729ad35 100644 --- a/charts/trino/README.md +++ b/charts/trino/README.md @@ -503,9 +503,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"` @@ -559,12 +561,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/charts/trino/templates/configmap-access-control.yaml b/charts/trino/templates/configmap-access-control-coordinator.yaml similarity index 100% rename from charts/trino/templates/configmap-access-control.yaml rename to charts/trino/templates/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 new file mode 100644 index 00000000..7585962b --- /dev/null +++ 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 index b1d4153a..13b656c9 100644 --- a/charts/trino/templates/configmap-coordinator.yaml +++ b/charts/trino/templates/configmap-coordinator.yaml @@ -77,6 +77,9 @@ data: jmx.rmiregistry.port={{- $coordinatorJmx.registryPort }} jmx.rmiserver.port={{- $coordinatorJmx.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 index 4a10dca9..ef7892ae 100644 --- a/charts/trino/templates/configmap-worker.yaml +++ b/charts/trino/templates/configmap-worker.yaml @@ -65,10 +65,19 @@ data: jmx.rmiregistry.port={{- $workerJmx.registryPort }} jmx.rmiserver.port={{- $workerJmx.serverPort }} {{- 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 index 8a54bc58..cca4c1ba 100644 --- a/charts/trino/templates/deployment-coordinator.yaml +++ b/charts/trino/templates/deployment-coordinator.yaml @@ -19,7 +19,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 index b04a5f39..da8d65c1 100644 --- a/charts/trino/templates/deployment-worker.yaml +++ b/charts/trino/templates/deployment-worker.yaml @@ -26,6 +26,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 }} @@ -61,6 +64,11 @@ spec: configMap: name: {{ template "trino.fullname" . }}-jmx-exporter-config-worker {{- end }} + {{- 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: @@ -92,7 +100,11 @@ spec: imagePullSecrets: {{- toYaml .Values.imagePullSecrets | nindent 8 }} {{- end }} + {{- 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" . }} @@ -112,6 +124,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 }} @@ -166,7 +182,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 $workerJmx.exporter.enabled }} diff --git a/charts/trino/templates/tests/test-graceful-shutdown.yaml b/charts/trino/templates/tests/test-graceful-shutdown.yaml new file mode 100644 index 00000000..79d93531 --- /dev/null +++ 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 index 7761ffcb..4009f129 100644 --- a/charts/trino/values.yaml +++ b/charts/trino/values.yaml @@ -586,9 +586,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: @@ -661,20 +663,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 new file mode 100644 index 00000000..87185484 --- /dev/null +++ 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 index afd3e5f4..0b208033 100755 --- 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' &2