From 383251f53b9ded6e37500ddddbe9063f4e314a67 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 17 Aug 2018 15:58:59 -0700 Subject: [PATCH] Fix kill timeout exceeding 5m on Docker driver Fixes an issue where the Docker API client would timeout before the kill timeout was hit. --- CHANGELOG.md | 2 ++ client/driver/docker.go | 2 +- client/driver/docker_test.go | 53 ++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22b3382a3ccd..0045b4043276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ IMPROVEMENTS: BUG FIXES: * core: Reset queued allocation summary to zero when job stopped [[GH-4414](https://github.com/hashicorp/nomad/issues/4414)] + * driver/docker: Fix kill timeout not being respected when timeout is over five + minutes [[GH-4599](https://github.com/hashicorp/nomad/issues/4599)] ## 0.8.4 (June 11, 2018) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6bebd0d6e3de..711d5c07581d 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -1913,7 +1913,7 @@ func (h *DockerHandle) Signal(s os.Signal) error { // Kill is used to terminate the task. This uses `docker stop -t killTimeout` func (h *DockerHandle) Kill() error { // Stop the container - err := h.client.StopContainer(h.containerID, uint(h.killTimeout.Seconds())) + err := h.waitClient.StopContainer(h.containerID, uint(h.killTimeout.Seconds())) if err != nil { h.executor.Exit() h.pluginClient.Kill() diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 04c74fdadb9c..207bbc1fc0b3 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -695,6 +695,59 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { } } +func TestDockerDriver_Start_KillTimeout(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + timeout := 2 * time.Second + task := &structs.Task{ + Name: "nc-demo", + Driver: "docker", + Config: map[string]interface{}{ + "image": "busybox", + "load": "busybox.tar", + "command": "/bin/sleep", + "args": []string{"10"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + KillTimeout: timeout, + KillSignal: "SIGUSR1", // Pick something that doesn't actually kill it + } + + _, handle, cleanup := dockerSetup(t, task) + defer cleanup() + + // Reduce the timeout for the docker client. + handle.client.SetTimeout(1 * time.Second) + + // Kill the task + var killSent, killed time.Time + go func() { + killSent = time.Now() + if err := handle.Kill(); err != nil { + t.Fatalf("err: %v", err) + } + }() + + select { + case <-handle.WaitCh(): + killed = time.Now() + case <-time.After(10 * time.Second): + t.Fatalf("timeout") + } + + if killed.Sub(killSent) < timeout { + t.Fatalf("kill timeout not respected") + } +} + func TestDockerDriver_StartN(t *testing.T) { if !tu.IsTravis() { t.Parallel()