Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check signal is unset before using user stopsignal #33335

Merged
merged 1 commit into from Jun 1, 2017

Conversation

@cpuguy83
Copy link
Contributor

commented May 22, 2017

This fixes an issue where if a stop signal is set, and a user sends
SIGKILL, container.ExitOnNext() is not set, thus causing the container
to restart.

Fixes #33334

@vdemeester

This comment has been minimized.

Copy link
Member

commented May 22, 2017

@cpuguy83 a test might need an update 👼 (I think because I think it was by design)

14:35:19 ----------------------------------------------------------------------
14:35:19 FAIL: docker_cli_kill_test.go:88: DockerSuite.TestKillWithStopSignalWithDifferentSignalShouldKeepRestartPolicy
14:35:19 
14:35:19 docker_cli_kill_test.go:98:
14:35:19     cli.WaitRestart(c, cid, 10*time.Second)
14:35:19 cli/cli.go:105:
14:35:19     t.Fatalf("condition \"%q == %q\" not true in time (%v)", out, expected, timeout)
14:35:19 ... Error: condition ""0" == "1"" not true in time (10s)
14:35:19 
14:35:19 
14:35:19 ----------------------------------------------------------------------

@cpuguy83 cpuguy83 force-pushed the cpuguy83:33334_check_unset_sig branch from cfb11cd to 4301a94 May 30, 2017

@cpuguy83

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2017

This is updated to only account for SIGKILL in this change to not change existing behavior.
Also adds a test.

Check signal is unset before using user stopsignal
This fixes an issue where if a stop signal is set, and a user sends
SIGKILL, `container.ExitOnNext()` is not set, thus causing the container
to restart.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

@cpuguy83 cpuguy83 force-pushed the cpuguy83:33334_check_unset_sig branch from 4301a94 to 114652a May 30, 2017

@thaJeztah thaJeztah referenced this pull request May 30, 2017
23 of 23 tasks complete
@boaz0
boaz0 approved these changes Jun 1, 2017
Copy link
Member

left a comment

👍

@thaJeztah
Copy link
Member

left a comment

LGTM

@vdemeester
Copy link
Member

left a comment

LGTM 🐯

@thaJeztah thaJeztah merged commit 872e28b into moby:master Jun 1, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 34557 has succeeded
Details
janky Jenkins build Docker-PRs 43155 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 3544 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 14435 has succeeded
Details
z Jenkins build Docker-PRs-s390x 3268 has succeeded
Details
@andrewhsu andrewhsu referenced this pull request Jun 6, 2017
40 of 40 tasks complete

@cpuguy83 cpuguy83 deleted the cpuguy83:33334_check_unset_sig branch Sep 20, 2017

liusdu pushed a commit to liusdu/moby that referenced this pull request Oct 30, 2017
Check signal is unset before using user stopsignal
This fixes an issue where if a stop signal is set, and a user sends
SIGKILL, `container.ExitOnNext()` is not set, thus causing the container
to restart.

backport from moby#33335
conflict:
	integration-cli/docker_api_containers_test.go

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
liusdu pushed a commit to liusdu/moby that referenced this pull request Oct 30, 2017
Merge branch 'check-unset-sig' into 'huawei-1.11.2'
Check signal is unset before using user stopsignal

This fixes an issue where if a stop signal is set, and a user sends
SIGKILL, `container.ExitOnNext()` is not set, thus causing the container
to restart.

backport from moby#33335
conflict:
	integration-cli/docker_api_containers_test.go

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>



See merge request docker/docker!634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.