strelok89 commented on code in PR #3855: URL: https://github.com/apache/celeborn/pull/3855#discussion_r4249161533
########## charts/celeborn/files/worker-drain.sh: ########## @@ -0,0 +1,130 @@ +#!/bin/sh +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# preStop hook for a Celeborn worker. Asks the worker to decommission when a scale-in is +# removing it, and to shut down gracefully otherwise. See the chart's README.md. +# +# Reads POD_NAME, POD_IP, STS_NAME, WORKER_HTTP_PORT and ALWAYS_DECOMMISSION from the +# environment. + +set -u + +SA=/var/run/secrets/kubernetes.io/serviceaccount + +# Kubernetes counts preStop against terminationGracePeriodSeconds, so no request may hang: a +# server that accepts the connection and then stops responding would burn the whole grace +# period and the pod would be killed without ever draining. Only the wait after a successful +# exit request is allowed to take that long. `-T` is the one timeout flag both busybox and GNU +# wget accept. +HTTP_TIMEOUT=10 +CURL_OPTS="--connect-timeout 5 --max-time $HTTP_TIMEOUT" +WGET_OPTS="-T $HTTP_TIMEOUT" + +# preStop output is not collected, so log through the main process. +log() { + _msg="celeborn preStop: $*" + if [ -w /proc/1/fd/1 ]; then + echo "$_msg" >/proc/1/fd/1 + else + echo "$_msg" + fi +} + +ORDINAL="" +DESIRED="" +EXIT_TYPE=GRACEFUL + +if [ "${ALWAYS_DECOMMISSION:-false}" = "true" ]; then + # Graceful shutdown persists state to recover from on restart. Where the worker's storage + # does not outlive the pod there is nothing to recover, so a rollout would drop in-flight + # shuffles - drain every time instead, whatever is removing this pod. + EXIT_TYPE=DECOMMISSION + log "alwaysDecommission is set - draining regardless of why this pod is going away" +else + # A scale-in must decommission, waiting for the shuffle data to expire. A rolling update or + # a node drain must not, or every pod replacement would block for hours. The statefulset + # controller lowers spec.replicas before deleting pods on a scale-in, so an ordinal at or + # above the desired count is being removed for good. + ORDINAL=${POD_NAME:-} + ORDINAL=${ORDINAL##*-} + + if [ -r "$SA/token" ]; then + URL="https://kubernetes.default.svc/apis/apps/v1/namespaces/$(cat "$SA/namespace")/statefulsets/${STS_NAME:-}/scale" + AUTH="Authorization: Bearer $(cat "$SA/token")" + if command -v curl >/dev/null 2>&1; then + SCALE=$(curl -sS $CURL_OPTS --cacert "$SA/ca.crt" -H "$AUTH" "$URL" 2>/dev/null) + else + SCALE=$(wget -q $WGET_OPTS -O - --ca-certificate="$SA/ca.crt" --header="$AUTH" "$URL" 2>/dev/null) + fi + DESIRED=$(printf '%s' "$SCALE" | sed 's/"status".*//' | grep -o '"replicas":[0-9 ]*' | head -n 1 | tr -dc '0-9') + fi + + case "$ORDINAL" in ''|*[!0-9]*) ORDINAL="" ;; esac + case "$DESIRED" in ''|*[!0-9]*) DESIRED="" ;; esac + + # Anything unreadable falls back to a graceful shutdown, so a failed lookup cannot stall a + # rollout with a drain that was never wanted. + if [ -n "$ORDINAL" ] && [ -n "$DESIRED" ] && [ "$ORDINAL" -ge "$DESIRED" ]; then + EXIT_TYPE=DECOMMISSION + fi +fi + +if [ "${ALWAYS_DECOMMISSION:-false}" != "true" ] && [ -z "$DESIRED" ]; then + log "could not read ${STS_NAME:-unknown} desired replicas - a scale-in will NOT decommission" +fi +log "ordinal=${ORDINAL:-unknown} desired=${DESIRED:-unknown} exit=$EXIT_TYPE" + +BODY='{"type":"'"$EXIT_TYPE"'"}' + +# The exit endpoint hands the drain to a separate thread and answers straight away, so bounding +# the request does not cut a drain short. +# The worker's HTTP server binds to one address, not the wildcard: celeborn.worker.http.host +# defaults to <localhost>, which resolves to this pod's own address, and Jetty is given that +# host. So loopback is refused - target the pod IP, and keep loopback only as a fallback for a +# deployment that has pointed the server somewhere else, such as 0.0.0.0. +HOSTS="" +if [ -n "${POD_IP:-}" ]; then + case "$POD_IP" in + *:*) HOSTS="[$POD_IP]" ;; + *) HOSTS="$POD_IP" ;; + esac +fi +HOSTS="$HOSTS 127.0.0.1" + +RC=1 +for HOST in $HOSTS; do + EXIT_URL="http://$HOST:${WORKER_HTTP_PORT:-9096}/api/v1/workers/exit"; Review Comment: Supporting the auth schemes would mean giving the hook its own credentials, and SPNEGO would need a Kerberos client in the hook, so I went with failing early. The chart now refuses to render `worker.drain` together with `celeborn.worker.http.ssl.enabled` or `celeborn.worker.http.auth.supportedSchemes`, and has tests for both. The README notes that the check only covers settings under `celeborn`. TLS support (https to the pod's own IP) could come in a follow-up if anyone needs it. ########## charts/celeborn/templates/worker/scaledobject.yaml: ########## @@ -0,0 +1,72 @@ +{{- /* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ -}} + +{{- if .Values.worker.autoscaling.enabled }} +{{- if not (or .Values.worker.autoscaling.diskUsage.enabled .Values.worker.autoscaling.memoryUsage.enabled .Values.worker.autoscaling.triggers) }} +{{- fail "worker.autoscaling.enabled is true but no trigger is: enable worker.autoscaling.diskUsage or memoryUsage, or set worker.autoscaling.triggers" }} +{{- end }} +{{- /* One ScaledObject per worker statefulset, so each zone scales on its own load. */}} +{{- $zones := list dict }} +{{- if .Values.worker.zoneAwareReplication.enabled }} +{{- $zones = .Values.worker.zoneAwareReplication.zones }} +{{- end }} +{{- range $index, $zone := $zones }} +{{- if $index }} +--- +{{- end }} +{{- with dict "Values" $.Values "Release" $.Release "Chart" $.Chart "Capabilities" $.Capabilities "Template" $.Template "Files" $.Files "zone" $zone }} +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{ include "celeborn.worker.statefulSet.name" . }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "celeborn.worker.labels" . | nindent 4 }} +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: StatefulSet + name: {{ include "celeborn.worker.statefulSet.name" . }} + {{- /* Not `default`: it treats an explicit 0 as unset, which rules out scaling to zero. */}} + minReplicaCount: {{ if kindIs "invalid" .Values.worker.autoscaling.minReplicaCount }}{{ include "celeborn.worker.replicas" . }}{{ else }}{{ .Values.worker.autoscaling.minReplicaCount }}{{ end }} Review Comment: Good catch. `minReplicaCount: 0` is now rejected unless `worker.autoscaling.triggers` has at least one entry. The README explains that this trigger has to fire without any workers running. The test now covers both the accepted case (with a cron trigger) and the rejected one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
