From e68c04b722b4c001c999abc2cd5b60b1f1b34457 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 11 Apr 2013 09:02:34 -0700 Subject: [PATCH 1/2] force kill now use lxc-kill. Fixes #383 --- container.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/container.go b/container.go index f180c7559b..06f39083d8 100644 --- a/container.go +++ b/container.go @@ -550,9 +550,21 @@ func (container *Container) kill() error { if !container.State.Running || container.cmd == nil { return nil } - if err := container.cmd.Process.Kill(); err != nil { - return err + + // Sending SIGINT to the process via lxc + output, err := exec.Command("lxc-kill", "-n", container.Id, "9").CombinedOutput() + if err != nil { + Debugf("error killing container %s (%s, %s)", container.Id, output, err) } + + // 2. Wait for the process to die, in last resort, try to kill the process directly + if err := container.WaitTimeout(10 * time.Second); err != nil { + log.Printf("Container %s failed to exit within 10 seconds of SIGINT - trying direct SIGKILL", container.Id) + if err := container.cmd.Process.Kill(); err != nil { + return err + } + } + // Wait for the container to be actually stopped container.Wait() return nil From bb22cd492efdc67855bbcf8ab4a669e45ea9ee8e Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 11 Apr 2013 16:21:19 -0700 Subject: [PATCH 2/2] Add unit test for hanging kill + fix other tests behaviour --- commands_test.go | 17 ++++++++++++++ container.go | 6 ++--- container_test.go | 56 +++++++++++++++++++++++++++++++++++++++++++++++ runtime_test.go | 1 - 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/commands_test.go b/commands_test.go index 30e2579d20..1be4a2abe0 100644 --- a/commands_test.go +++ b/commands_test.go @@ -59,6 +59,20 @@ func assertPipe(input, output string, r io.Reader, w io.Writer, count int) error return nil } +func cmdWait(srv *Server, container *Container) error { + stdout, stdoutPipe := io.Pipe() + + go func() { + srv.CmdWait(nil, stdoutPipe, container.Id) + }() + + if _, err := bufio.NewReader(stdout).ReadString('\n'); err != nil { + return err + } + // Cleanup pipes + return closeWrap(stdout, stdoutPipe) +} + // TestRunHostname checks that 'docker run -h' correctly sets a custom hostname func TestRunHostname(t *testing.T) { runtime, err := newTestRuntime() @@ -89,7 +103,9 @@ func TestRunHostname(t *testing.T) { setTimeout(t, "CmdRun timed out", 2*time.Second, func() { <-c + cmdWait(srv, srv.runtime.List()[0]) }) + } func TestRunExit(t *testing.T) { @@ -129,6 +145,7 @@ func TestRunExit(t *testing.T) { // as the process exited, CmdRun must finish and unblock. Wait for it setTimeout(t, "Waiting for CmdRun timed out", 2*time.Second, func() { <-c1 + cmdWait(srv, container) }) // Make sure that the client has been disconnected diff --git a/container.go b/container.go index 06f39083d8..d644ac7115 100644 --- a/container.go +++ b/container.go @@ -551,15 +551,15 @@ func (container *Container) kill() error { return nil } - // Sending SIGINT to the process via lxc + // Sending SIGKILL to the process via lxc output, err := exec.Command("lxc-kill", "-n", container.Id, "9").CombinedOutput() if err != nil { - Debugf("error killing container %s (%s, %s)", container.Id, output, err) + log.Printf("error killing container %s (%s, %s)", container.Id, output, err) } // 2. Wait for the process to die, in last resort, try to kill the process directly if err := container.WaitTimeout(10 * time.Second); err != nil { - log.Printf("Container %s failed to exit within 10 seconds of SIGINT - trying direct SIGKILL", container.Id) + log.Printf("Container %s failed to exit within 10 seconds of lxc SIGKILL - trying direct SIGKILL", container.Id) if err := container.cmd.Process.Kill(); err != nil { return err } diff --git a/container_test.go b/container_test.go index ac47f84bf0..d5f3694c59 100644 --- a/container_test.go +++ b/container_test.go @@ -324,6 +324,54 @@ func TestOutput(t *testing.T) { } } +func TestKillDifferentUser(t *testing.T) { + runtime, err := newTestRuntime() + if err != nil { + t.Fatal(err) + } + defer nuke(runtime) + container, err := runtime.Create(&Config{ + Image: GetTestImage(runtime).Id, + Cmd: []string{"tail", "-f", "/etc/resolv.conf"}, + User: "daemon", + }, + ) + if err != nil { + t.Fatal(err) + } + defer runtime.Destroy(container) + + if container.State.Running { + t.Errorf("Container shouldn't be running") + } + if err := container.Start(); err != nil { + t.Fatal(err) + } + + // Give some time to lxc to spawn the process (setuid might take some time) + container.WaitTimeout(500 * time.Millisecond) + + if !container.State.Running { + t.Errorf("Container should be running") + } + + if err := container.Kill(); err != nil { + t.Fatal(err) + } + + if container.State.Running { + t.Errorf("Container shouldn't be running") + } + container.Wait() + if container.State.Running { + t.Errorf("Container shouldn't be running") + } + // Try stopping twice + if err := container.Kill(); err != nil { + t.Fatal(err) + } +} + func TestKill(t *testing.T) { runtime, err := newTestRuntime() if err != nil { @@ -346,6 +394,10 @@ func TestKill(t *testing.T) { if err := container.Start(); err != nil { t.Fatal(err) } + + // Give some time to lxc to spawn the process + container.WaitTimeout(500 * time.Millisecond) + if !container.State.Running { t.Errorf("Container should be running") } @@ -657,6 +709,10 @@ func TestMultipleContainers(t *testing.T) { t.Fatal(err) } + // Make sure they are running before trying to kill them + container1.WaitTimeout(250 * time.Millisecond) + container2.WaitTimeout(250 * time.Millisecond) + // If we are here, both containers should be running if !container1.State.Running { t.Fatal("Container not running") diff --git a/runtime_test.go b/runtime_test.go index 9ab8b9b1e7..55d2b54e7e 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -17,7 +17,6 @@ const testLayerPath string = "/var/lib/docker/docker-ut.tar" const unitTestImageName string = "docker-ut" var unitTestStoreBase string -var srv *Server func nuke(runtime *Runtime) error { var wg sync.WaitGroup