Skip to content

Make docker stop timeout configurable via STOP_TIMEOUT property#743

Open
DaveDasSchnitzel wants to merge 1 commit intojenkinsci:masterfrom
DaveDasSchnitzel:feature/configurable-timeout
Open

Make docker stop timeout configurable via STOP_TIMEOUT property#743
DaveDasSchnitzel wants to merge 1 commit intojenkinsci:masterfrom
DaveDasSchnitzel:feature/configurable-timeout

Conversation

@DaveDasSchnitzel
Copy link
Copy Markdown

@DaveDasSchnitzel DaveDasSchnitzel commented Mar 25, 2026

The --time argument to docker stop was previously hardcoded and could
not be adjusted for containers that need more time to shut down gracefully.
Modern Docker has also deprecated --time in favour of --timeout, though
the --timeout long form was only introduced in Docker 23 and is not
recognised by older clients.

Add a static STOP_TIMEOUT field consistent with the existing
CLIENT_TIMEOUT and SKIP_RM_ON_STOP fields, and switch to the short
-t flag which maps to the timeout parameter on all Docker client versions
without deprecation warnings. The value can be overridden via the system
property:
org.jenkinsci.plugins.docker.workflow.client.DockerClient.STOP_TIMEOUT
or mutated from a Groovy init script at runtime.

Fixes #733
Related to #719

Testing done

Ran DockerClientTest#test_run against a live Docker daemon with the default timeout (1 s)
and with an override of 5 s.

Default (STOP_TIMEOUT=1):

$ docker run -t -d -u 1000:1000 docker.io/library/docker@sha256:d842418d21545fde57c2512681d9bdc4ce0e54f2e0305a293ee20a9b6166932b cat
$ docker stop -t 1 091b9b67a08261b65a7e6f99ba1c8dba0845acff33b2d5a764e0d4cc68f4555d
$ docker rm -f --volumes 091b9b67a08261b65a7e6f99ba1c8dba0845acff33b2d5a764e0d4cc68f4555d
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.248 s

Override (-Dorg.jenkinsci.plugins.docker.workflow.client.DockerClient.STOP_TIMEOUT=5):

$ docker run -t -d -u 1000:1000 docker.io/library/docker@sha256:d842418d21545fde57c2512681d9bdc4ce0e54f2e0305a293ee20a9b6166932b cat
$ docker stop -t 5 c8591c3f5f5757389f98b7e24724df6879ca792652fc196191937fba3d9ce06e
$ docker rm -f --volumes c8591c3f5f5757389f98b7e24724df6879ca792652fc196191937fba3d9ce06e
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.871 s

Submitter checklist

The --time argument to `docker stop` was previously hardcoded and could
not be adjusted for containers that need more time to shut down gracefully.
Modern Docker has also deprecated --time in favour of --timeout, though
the --timeout long form was only introduced in Docker 23 and is not
recognised by older clients.

Add a static STOP_TIMEOUT field consistent with the existing
CLIENT_TIMEOUT and SKIP_RM_ON_STOP fields, and switch to the short -t
flag which maps to the timeout parameter on all Docker client versions
without deprecation warnings. The value can be overridden via the system
property:
  org.jenkinsci.plugins.docker.workflow.client.DockerClient.STOP_TIMEOUT
or mutated from a Groovy init script at runtime.

Fixes jenkinsci#733
Related to jenkinsci#719
@DaveDasSchnitzel DaveDasSchnitzel force-pushed the feature/configurable-timeout branch from 171cb28 to 973d288 Compare March 26, 2026 08:06
@DaveDasSchnitzel
Copy link
Copy Markdown
Author

Hey @jglick

I noticed this plugin is listed for adoption on the Jenkins plugin site. I've opened this PR to fix a issue where the docker stop timeout is hardcoded and can't be adjusted for containers that need more time to shut down gracefully.

Would you have a chance to review this, or let me know if you're still actively maintaining the plugin? If not, I'd be happy to go through the adoption process to help keep it moving forward.

Thank you for all your work on this plugin. :-)

@jglick
Copy link
Copy Markdown
Member

jglick commented Mar 27, 2026

let me know if you're still actively maintaining the plugin?

I am not. On occasion I merge PRs to adapt tests to behavioral changes elsewhere, consume better upstream APIs, update metadata, etc.

In general my opinion is that it is not safe or prudent to make any substantive changes to this plugin ever again (excepting those required by OS or Docker updates), and users should consider it “as is”: to the extent it works and is convenient, fine, but at the first sign of trouble, stop using it and run Docker tools directly. If someone wants to really adopt it and take responsibility for triaging and fixing the inevitable regressions caused by such changes, I guess they should. Other than the obvious Docker expertise, the level of Jenkins plugin development expertise required is I would say intermediate: there are some subtle aspects particularly to WithContainerStep.Decorator. Since the plugin is still pretty widely used I would expect any maintainer to be someone with at least a couple years of proven experience contributing to the Jenkins ecosystem.

@DaveDasSchnitzel
Copy link
Copy Markdown
Author

let me know if you're still actively maintaining the plugin?

I am not. On occasion I merge PRs to adapt tests to behavioral changes elsewhere, consume better upstream APIs, update metadata, etc.

Thanks for the clarification. That helps a lot, we’ll reconsider how we approach this.

@antovic
Copy link
Copy Markdown

antovic commented Apr 6, 2026

Since timeout can be configured when a container is created, would it be acceptable to remove the deprecated --time flag?

image Source: https://docs.docker.com/reference/cli/docker/container/stop/

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.

[JENKINS-76036] Flag --time has been deprecated, use --timeout instead

4 participants