Skip to content

Add a stop_timeout option#1813

Open
djmb wants to merge 1 commit intomainfrom
stop-timeout
Open

Add a stop_timeout option#1813
djmb wants to merge 1 commit intomainfrom
stop-timeout

Conversation

@djmb
Copy link
Copy Markdown
Collaborator

@djmb djmb commented Apr 2, 2026

The drain_timeout option is used to allow in flight requests to finish.

For proxied roles draining is handled by the proxy deploy command - once it starts no new requests are sent to the old container. Then when calling docker stop we can use a short timeout (we use the default of 10s, but really it should be shorter).

For non-proxied roles calling docker stop initially sends a SIGTERM and we assume that will start the drain process, so we use drain timeout as the timeout there.

If you are running proxy + non-proxied workloads in a single container, then the default behaviour means that the non-proxied workload will get a 10 second timeout via docker stop, which may not be enough.

Add a stop_timeout option that allows you to configure the timeout used for docker stop. By default it will use the drain timeout for non-proxied roles and 10s for proxied roles, but you can override it globally or per role.

The `drain_timeout` option is used to allow in flight requests to
finish.

For proxied roles draining is handled by the proxy deploy command - once
it starts no new requests are sent to the old container. Then when
calling `docker stop` we can use a short timeout (we use the default
of 10s, but really it should be shorter).

For non-proxied roles calling `docker stop` initially sends a SIGTERM
and we assume that will start the drain process, so we use drain timeout
as the timeout there.

If you are running proxy + non-proxied workloads in a single container,
then the default behaviour means that the non-proxied workload will get
a 10 second timeout via `docker stop`, which may not be enough.

Add a `stop_timeout` option that allows you to configure the timeout
used for `docker stop`. By default it will use the drain timeout for
non-proxied roles and 10s for proxied roles, but you can override it
globally or per role.
Copilot AI review requested due to automatic review settings April 2, 2026 07:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a configurable stop_timeout setting to control the timeout passed to docker stop, with sensible defaults based on whether a role is proxied (Docker default 10s) or non-proxied (uses drain_timeout), and supports both global and per-role overrides.

Changes:

  • Add stop_timeout support at the root configuration level and per-role specialization.
  • Update role stop argument generation to prefer stop_timeout when present.
  • Extend documentation examples and tests to cover the new option and precedence rules.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/configuration/role_test.rb Adds unit tests validating stop_timeout behavior and override precedence (role vs root).
test/commands/app_test.rb Adds command-generation tests asserting docker stop -t <N> is emitted when configured.
lib/kamal/configuration/role.rb Implements Role#stop_timeout and updates stop_args to use it.
lib/kamal/configuration/docs/role.yml Documents stop_timeout as a per-role option in the role config examples.
lib/kamal/configuration/docs/configuration.yml Documents stop_timeout at the root level and describes default behavior.
lib/kamal/configuration.rb Exposes Configuration#stop_timeout from raw config for role-level consumption.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants