Skip to content

Commit 21dc31c

Browse files
committed
refactor: wait-shutdown preStop hook is not necessary
/wait-shutdown preStop script's only job is to send SIGTERM to nginx-ingress-controller, which is PID 1, so it's the same with or without in Kubernetes environments. See #6287 for discussion.
1 parent 2aa0ec7 commit 21dc31c

File tree

7 files changed

+4
-72
lines changed

7 files changed

+4
-72
lines changed

build/build.sh

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,3 @@ ${GO_BUILD_CMD} \
5757
-X ${PKG}/version.REPO=${REPO_INFO}" \
5858
-buildvcs=false \
5959
-o "${TARGETS_DIR}/dbg" "${PKG}/cmd/dbg"
60-
61-
echo "Building ${PKG}/cmd/waitshutdown"
62-
63-
${GO_BUILD_CMD} \
64-
-trimpath -ldflags="-buildid= -w -s \
65-
-X ${PKG}/version.RELEASE=${TAG} \
66-
-X ${PKG}/version.COMMIT=${COMMIT_SHA} \
67-
-X ${PKG}/version.REPO=${REPO_INFO}" \
68-
-buildvcs=false \
69-
-o "${TARGETS_DIR}/wait-shutdown" "${PKG}/cmd/waitshutdown"

charts/ingress-nginx/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ metadata:
372372
| controller.keda.triggers | list | `[]` | |
373373
| controller.kind | string | `"Deployment"` | Use a `DaemonSet` or `Deployment` |
374374
| controller.labels | object | `{}` | Labels to be added to the controller Deployment or DaemonSet and other resources that do not have option to specify labels # |
375-
| controller.lifecycle | object | `{"preStop":{"exec":{"command":["/wait-shutdown"]}}}` | Improve connection draining when ingress controller pod is deleted using a lifecycle hook: With this new hook, we increased the default terminationGracePeriodSeconds from 30 seconds to 300, allowing the draining of connections up to five minutes. If the active connections end before that, the pod will terminate gracefully at that time. To effectively take advantage of this feature, the Configmap feature worker-shutdown-timeout new value is 240s instead of 10s. # |
375+
| controller.lifecycle | object | `{}` | |
376376
| controller.livenessProbe.failureThreshold | int | `5` | |
377377
| controller.livenessProbe.httpGet.path | string | `"/healthz"` | |
378378
| controller.livenessProbe.httpGet.port | int | `10254` | |

charts/ingress-nginx/values.yaml

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -945,18 +945,7 @@ controller:
945945
# annotations:
946946
# description: Too many 4XXs
947947
# summary: More than 5% of all requests returned 4XX, this requires your attention
948-
# -- Improve connection draining when ingress controller pod is deleted using a lifecycle hook:
949-
# With this new hook, we increased the default terminationGracePeriodSeconds from 30 seconds
950-
# to 300, allowing the draining of connections up to five minutes.
951-
# If the active connections end before that, the pod will terminate gracefully at that time.
952-
# To effectively take advantage of this feature, the Configmap feature
953-
# worker-shutdown-timeout new value is 240s instead of 10s.
954-
##
955-
lifecycle:
956-
preStop:
957-
exec:
958-
command:
959-
- /wait-shutdown
948+
lifecycle: {}
960949
priorityClassName: ""
961950
# -- Rollback limit
962951
##

cmd/waitshutdown/main.go

Lines changed: 0 additions & 43 deletions
This file was deleted.

rootfs/Dockerfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ COPY --chown=www-data:www-data etc /etc
4343

4444
COPY --chown=www-data:www-data bin/${TARGETARCH}/dbg /
4545
COPY --chown=www-data:www-data bin/${TARGETARCH}/nginx-ingress-controller /
46-
COPY --chown=www-data:www-data bin/${TARGETARCH}/wait-shutdown /
4746

4847
# Fix permission during the build to avoid issues at runtime
4948
# with volumes (custom templates)

rootfs/Dockerfile-chroot

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ COPY --chown=www-data:www-data etc /chroot/etc
6565

6666
COPY --chown=www-data:www-data bin/${TARGETARCH}/dbg /
6767
COPY --chown=www-data:www-data bin/${TARGETARCH}/nginx-ingress-controller /
68-
COPY --chown=www-data:www-data bin/${TARGETARCH}/wait-shutdown /
6968
COPY --chown=www-data:www-data nginx-chroot-wrapper.sh /usr/bin/nginx
7069

7170
WORKDIR /chroot/etc/nginx

test/e2e/gracefulshutdown/grace_period.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,14 @@ var _ = framework.IngressNginxDescribe("[Shutdown] Grace period shutdown", func(
4141
if strings.Contains(v, "--shutdown-grace-period") {
4242
continue
4343
}
44-
4544
args = append(args, v)
4645
}
47-
4846
args = append(args, "--shutdown-grace-period=90")
4947
deployment.Spec.Template.Spec.Containers[0].Args = args
50-
cmds := []string{"/wait-shutdown"}
51-
deployment.Spec.Template.Spec.Containers[0].Lifecycle.PreStop.Exec.Command = cmds
48+
5249
grace := int64(3600)
5350
deployment.Spec.Template.Spec.TerminationGracePeriodSeconds = &grace
51+
5452
_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
5553
return err
5654
})

0 commit comments

Comments
 (0)