From 1161dda05e962ef4b62a51182faf5dd38b72d7a2 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 27 Apr 2026 18:18:39 -0400 Subject: [PATCH] feat: send SIGTERM to old containers immediately on deploy The docker-local scheduler now sends `SIGTERM` to old containers immediately after a successful deploy via `docker container kill --signal=SIGTERM`, rather than waiting `wait-to-retire` seconds before signaling. This matches Heroku's graceful-shutdown contract and lets applications begin draining in-flight work as soon as proxy traffic switches. The existing `wait-to-retire` grace period and `stop-timeout-seconds` hard-stop continue to apply unchanged as the authoritative cleanup path. --- docs/appendices/0.38.0-migration-guide.md | 1 + docs/deployment/zero-downtime-deploys.md | 4 +- .../scheduler-docker-local/scheduler-deploy | 13 +++++++ tests/unit/scheduler-docker-local.bats | 38 +++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/docs/appendices/0.38.0-migration-guide.md b/docs/appendices/0.38.0-migration-guide.md index 5ff39fd7e..4c809cf6e 100644 --- a/docs/appendices/0.38.0-migration-guide.md +++ b/docs/appendices/0.38.0-migration-guide.md @@ -18,6 +18,7 @@ - The path on disk to both the global `ENV` file and app `ENV` files have been moved. Users should reference environment variables via the provided plugin triggers rather than directly sourcing the ENV files. Existing ENV files are left untouched and will be removed on the subsequent Dokku install. - During a fresh apt install, the upstream nginx default vhost files (`/etc/nginx/sites-enabled/default`, `/etc/nginx/sites-available/default`, and `/etc/nginx/conf.d/default.conf`) are renamed to `${path}.dokku-disabled` (not deleted) to avoid a `duplicate default server for 0.0.0.0:80` error. Operators with local customizations can recover them by inspecting the `.dokku-disabled` siblings. Upgrade-in-place installs do not touch any existing nginx files. - Fresh apt installs now ship a catch-all default site at `/etc/nginx/conf.d/00-default-vhost.conf` that rejects requests with unknown Host headers using `ssl_reject_handshake on` (HTTPS) and `return 444` (HTTP). This replaces the manual workaround previously documented in the nginx docs. The behavior can be opted out at install time via the `dokku/install_default_site` debconf prompt. See the [Default site documentation](/docs/networking/proxies/nginx.md#default-site). +- The `docker-local` scheduler now sends `SIGTERM` to old containers immediately after a successful deploy, rather than waiting `wait-to-retire` seconds before signaling. This matches Heroku's graceful-shutdown contract and lets applications begin draining in-flight work as soon as proxy traffic switches. The `wait-to-retire` grace period and `stop-timeout-seconds` hard-stop continue to apply as before. See the [zero downtime deploys documentation](/docs/deployment/zero-downtime-deploys.md#wait-to-retire) for more details. ### TLS handshake behavior change diff --git a/docs/deployment/zero-downtime-deploys.md b/docs/deployment/zero-downtime-deploys.md index 5ef21b4ee..56bb1cf23 100644 --- a/docs/deployment/zero-downtime-deploys.md +++ b/docs/deployment/zero-downtime-deploys.md @@ -28,7 +28,9 @@ You may both create user-defined checks for web processes using the `healthcheck ### wait-to-retire -After a successful deploy, the grace period given to old containers before they are stopped/terminated is determined by the value of `wait-to-retire`. This is useful for ensuring completion of long-running HTTP connections. +After a successful deploy, Dokku immediately sends `SIGTERM` to old containers so the application can begin a graceful shutdown (draining in-flight connections, flushing buffers, and so on) the moment proxy traffic switches to the new containers. The old containers are then left running for the `wait-to-retire` grace period before being stopped, which sends `SIGTERM` again and ultimately `SIGKILL` after `stop-timeout-seconds`. This mirrors Heroku's [dyno shutdown behavior](https://devcenter.heroku.com/articles/dyno-shutdown-behavior). + +The `wait-to-retire` value controls the grace period and is useful for ensuring completion of long-running HTTP connections. ```shell dokku checks:set node-js-app wait-to-retire 30 diff --git a/plugins/scheduler-docker-local/scheduler-deploy b/plugins/scheduler-docker-local/scheduler-deploy index 7981c5a7f..16c3370be 100755 --- a/plugins/scheduler-docker-local/scheduler-deploy +++ b/plugins/scheduler-docker-local/scheduler-deploy @@ -84,6 +84,19 @@ trigger-scheduler-docker-local-scheduler-deploy() { # kill the old container if [[ -n "$oldids" ]]; then + # Send SIGTERM to old containers immediately so the application can begin + # graceful shutdown the moment proxy traffic switches to the new containers. + # We use `container kill --signal=SIGTERM` rather than `container stop` + # because it returns immediately and docker does not follow up with a + # SIGKILL on its own when the signal is something other than SIGKILL - + # the existing background block below remains responsible for the + # eventual hard stop after the wait-to-retire grace period. + for oldid in $oldids; do + if "$DOCKER_BIN" container inspect "$oldid" &>/dev/null; then + "$DOCKER_BIN" container kill --signal=SIGTERM "$oldid" &>/dev/null || true + fi + done + # Let the old container finish processing requests, before terminating it dokku_log_info1 "Shutting down old containers in $DOKKU_WAIT_TO_RETIRE seconds" ( diff --git a/tests/unit/scheduler-docker-local.bats b/tests/unit/scheduler-docker-local.bats index fe9e9531d..1260c127e 100644 --- a/tests/unit/scheduler-docker-local.bats +++ b/tests/unit/scheduler-docker-local.bats @@ -176,6 +176,44 @@ teardown() { assert_output_contains "Deploys may fail when publishing ports and enabling zero downtime" 0 } +@test "(scheduler-docker-local) sends SIGTERM immediately on deploy" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku checks:set $TEST_APP wait-to-retire 600" + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app + echo "output: $output" + echo "status: $status" + assert_success + + local old_cid + old_cid="$(docker ps --filter label=com.dokku.app-name=$TEST_APP --filter label=com.dokku.process-type=web --format '{{.ID}}')" + [[ -n "$old_cid" ]] + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + sleep 5 + + run /bin/bash -c "docker inspect --format '{{.State.Status}}' $old_cid" + echo "output: $output" + echo "status: $status" + assert_output "exited" + + run /bin/bash -c "docker ps --filter label=com.dokku.app-name=$TEST_APP --filter label=com.dokku.process-type=web --format '{{.Names}}'" + echo "output: $output" + echo "status: $status" + assert_output "$TEST_APP.web.1" +} + @test "(scheduler-docker-local) scale down retires orphaned containers" { run create_app echo "output: $output"