Skip to content

Commit bc503ca

Browse files
author
John Howard
committed
Windows: [TP4] docker kill handling
Signed-off-by: John Howard <[email protected]>
1 parent 2465ab8 commit bc503ca

26 files changed

+186
-175
lines changed

‎daemon/execdriver/windows/exec.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo
1818
term execdriver.Terminal
1919
err error
2020
exitCode int32
21+
errno uint32
2122
)
2223

2324
active := d.activeContainers[c.ID]
@@ -77,12 +78,15 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo
7778
hooks.Start(&c.ProcessConfig, int(pid), chOOM)
7879
}
7980

80-
if exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid); err != nil {
81-
logrus.Errorf("Failed to WaitForProcessInComputeSystem %s", err)
81+
if exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite); err != nil {
82+
if errno == hcsshim.Win32PipeHasBeenEnded {
83+
logrus.Debugf("Exiting Run() after WaitForProcessInComputeSystem failed with recognised error 0x%X", errno)
84+
return hcsshim.WaitErrExecFailed, nil
85+
}
86+
logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): 0x%X %s", errno, err)
8287
return -1, err
8388
}
8489

85-
// TODO Windows - Do something with this exit code
86-
logrus.Debugln("Exiting Run() with ExitCode 0", c.ID)
90+
logrus.Debugln("Exiting Run()", c.ID)
8791
return int(exitCode), nil
8892
}

‎daemon/execdriver/windows/run.go‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -220,22 +220,19 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd
220220
}
221221
defer func() {
222222
// Stop the container
223-
224-
if terminateMode {
225-
logrus.Debugf("Terminating container %s", c.ID)
226-
if err := hcsshim.TerminateComputeSystem(c.ID); err != nil {
227-
// IMPORTANT: Don't fail if fails to change state. It could already
228-
// have been stopped through kill().
229-
// Otherwise, the docker daemon will hang in job wait()
230-
logrus.Warnf("Ignoring error from TerminateComputeSystem %s", err)
223+
if forceKill {
224+
logrus.Debugf("Forcibly terminating container %s", c.ID)
225+
if errno, err := hcsshim.TerminateComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
226+
logrus.Warnf("Ignoring error from TerminateComputeSystem 0x%X %s", errno, err)
231227
}
232228
} else {
233229
logrus.Debugf("Shutting down container %s", c.ID)
234-
if err := hcsshim.ShutdownComputeSystem(c.ID); err != nil {
235-
// IMPORTANT: Don't fail if fails to change state. It could already
236-
// have been stopped through kill().
237-
// Otherwise, the docker daemon will hang in job wait()
238-
logrus.Warnf("Ignoring error from ShutdownComputeSystem %s", err)
230+
if errno, err := hcsshim.ShutdownComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
231+
if errno != hcsshim.Win32SystemShutdownIsInProgress &&
232+
errno != hcsshim.Win32SpecifiedPathInvalid &&
233+
errno != hcsshim.Win32SystemCannotFindThePathSpecified {
234+
logrus.Warnf("Ignoring error from ShutdownComputeSystem 0x%X %s", errno, err)
235+
}
239236
}
240237
}
241238
}()
@@ -303,11 +300,20 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd
303300
hooks.Start(&c.ProcessConfig, int(pid), chOOM)
304301
}
305302

306-
var exitCode int32
307-
exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid)
303+
var (
304+
exitCode int32
305+
errno uint32
306+
)
307+
exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite)
308308
if err != nil {
309-
logrus.Errorf("Failed to WaitForProcessInComputeSystem %s", err)
310-
return execdriver.ExitStatus{ExitCode: -1}, err
309+
if errno != hcsshim.Win32PipeHasBeenEnded {
310+
logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err)
311+
}
312+
// Do NOT return err here as the container would have
313+
// started, otherwise docker will deadlock. It's perfectly legitimate
314+
// for WaitForProcessInComputeSystem to fail in situations such
315+
// as the container being killed on another thread.
316+
return execdriver.ExitStatus{ExitCode: hcsshim.WaitErrExecFailed}, nil
311317
}
312318

313319
logrus.Debugf("Exiting Run() exitCode %d id=%s", exitCode, c.ID)

‎daemon/execdriver/windows/terminatekill.go‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,46 @@
33
package windows
44

55
import (
6+
"fmt"
7+
"syscall"
8+
69
"github.com/Sirupsen/logrus"
710
"github.com/docker/docker/daemon/execdriver"
811
"github.com/microsoft/hcsshim"
912
)
1013

1114
// Terminate implements the exec driver Driver interface.
1215
func (d *Driver) Terminate(p *execdriver.Command) error {
13-
logrus.Debugf("WindowsExec: Terminate() id=%s", p.ID)
14-
return kill(p.ID, p.ContainerPid)
16+
return kill(p.ID, p.ContainerPid, syscall.SIGTERM)
1517
}
1618

1719
// Kill implements the exec driver Driver interface.
1820
func (d *Driver) Kill(p *execdriver.Command, sig int) error {
19-
logrus.Debugf("WindowsExec: Kill() id=%s sig=%d", p.ID, sig)
20-
return kill(p.ID, p.ContainerPid)
21+
return kill(p.ID, p.ContainerPid, syscall.Signal(sig))
2122
}
2223

23-
func kill(id string, pid int) error {
24-
logrus.Debugln("kill() ", id, pid)
24+
func kill(id string, pid int, sig syscall.Signal) error {
25+
logrus.Debugf("WindowsExec: kill() id=%s pid=%d sig=%d", id, pid, sig)
2526
var err error
27+
context := fmt.Sprintf("kill: sig=%d pid=%d", sig, pid)
2628

27-
// Terminate Process
28-
if err = hcsshim.TerminateProcessInComputeSystem(id, uint32(pid)); err != nil {
29-
logrus.Warnf("Failed to terminate pid %d in %s: %q", pid, id, err)
30-
// Ignore errors
31-
err = nil
32-
}
33-
34-
if terminateMode {
29+
if sig == syscall.SIGKILL || forceKill {
3530
// Terminate the compute system
36-
if err = hcsshim.TerminateComputeSystem(id); err != nil {
37-
logrus.Errorf("Failed to terminate %s - %q", id, err)
31+
if errno, err := hcsshim.TerminateComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
32+
logrus.Errorf("Failed to terminate %s - 0x%X %q", id, errno, err)
3833
}
3934

4035
} else {
36+
// Terminate Process
37+
if err = hcsshim.TerminateProcessInComputeSystem(id, uint32(pid)); err != nil {
38+
logrus.Warnf("Failed to terminate pid %d in %s: %q", pid, id, err)
39+
// Ignore errors
40+
err = nil
41+
}
42+
4143
// Shutdown the compute system
42-
if err = hcsshim.ShutdownComputeSystem(id); err != nil {
43-
logrus.Errorf("Failed to shutdown %s - %q", id, err)
44+
if errno, err := hcsshim.ShutdownComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
45+
logrus.Errorf("Failed to shutdown %s - 0x%X %q", id, errno, err)
4446
}
4547
}
4648
return err

‎daemon/execdriver/windows/windows.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import (
1818
var dummyMode bool
1919

2020
// This allows the daemon to terminate containers rather than shutdown
21-
var terminateMode bool
21+
// This allows the daemon to force kill (HCS terminate) rather than shutdown
22+
var forceKill bool
2223

2324
// Define name and version for windows
2425
var (
@@ -62,11 +63,11 @@ func NewDriver(root, initPath string, options []string) (*Driver, error) {
6263
logrus.Warn("Using dummy mode in Windows exec driver. This is for development use only!")
6364
}
6465

65-
case "terminate":
66+
case "forcekill":
6667
switch val {
6768
case "1":
68-
terminateMode = true
69-
logrus.Warn("Using terminate mode in Windows exec driver. This is for testing purposes only.")
69+
forceKill = true
70+
logrus.Warn("Using force kill mode in Windows exec driver. This is for testing purposes only.")
7071
}
7172

7273
default:

‎daemon/kill.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
package daemon
22

3-
import "syscall"
3+
import (
4+
"fmt"
5+
"runtime"
6+
"syscall"
7+
8+
"github.com/docker/docker/pkg/signal"
9+
)
410

511
// ContainerKill send signal to the container
612
// If no signal is given (sig 0), then Kill with SIGKILL and wait
@@ -12,6 +18,10 @@ func (daemon *Daemon) ContainerKill(name string, sig uint64) error {
1218
return err
1319
}
1420

21+
if sig != 0 && !signal.ValidSignalForPlatform(syscall.Signal(sig)) {
22+
return fmt.Errorf("The %s daemon does not support signal %d", runtime.GOOS, sig)
23+
}
24+
1525
// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
1626
if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
1727
if err := container.Kill(); err != nil {

‎hack/vendor.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ clone git github.com/gorilla/context 14f550f51a
1414
clone git github.com/gorilla/mux e444e69cbd
1515
clone git github.com/kr/pty 5cf931ef8f
1616
clone git github.com/mattn/go-sqlite3 v1.1.0
17-
clone git github.com/microsoft/hcsshim 7f646aa6b26bcf90caee91e93cde4a80d0d8a83e
17+
clone git github.com/microsoft/hcsshim 325e531f8c49dd78580d5fd197ddb972fa4610e7
1818
clone git github.com/mistifyio/go-zfs v2.1.1
1919
clone git github.com/tchap/go-patricia v2.1.0
2020
clone git golang.org/x/net 3cffabab72adf04f8e3b01c5baf775361837b5fe https://github.com/golang/net.git

‎pkg/signal/signal.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,13 @@ func ParseSignal(rawSignal string) (syscall.Signal, error) {
4242
}
4343
return signal, nil
4444
}
45+
46+
// ValidSignalForPlatform returns true if a signal is valid on the platform
47+
func ValidSignalForPlatform(sig syscall.Signal) bool {
48+
for _, v := range SignalMap {
49+
if v == sig {
50+
return true
51+
}
52+
}
53+
return false
54+
}

‎vendor/src/github.com/microsoft/hcsshim/activatelayer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
// An activated layer must later be deactivated via DeactivateLayer.
1515
func ActivateLayer(info DriverInfo, id string) error {
1616
title := "hcsshim::ActivateLayer "
17-
logrus.Debugf(title+"Flavour %s ID %s", info.Flavour, id)
17+
logrus.Debugf(title+"Flavour %d ID %s", info.Flavour, id)
1818

1919
// Load the DLL and get a handle to the procedure we need
2020
dll, proc, err := loadAndFind(procActivateLayer)

‎vendor/src/github.com/microsoft/hcsshim/createlayer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
// the parent layer provided.
1313
func CreateLayer(info DriverInfo, id, parent string) error {
1414
title := "hcsshim::CreateLayer "
15-
logrus.Debugf(title+"Flavour %s ID %s parent %s", info.Flavour, id, parent)
15+
logrus.Debugf(title+"Flavour %d ID %s parent %s", info.Flavour, id, parent)
1616

1717
// Load the DLL and get a handle to the procedure we need
1818
dll, proc, err := loadAndFind(procCreateLayer)

‎vendor/src/github.com/microsoft/hcsshim/createprocess.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (p *pipe) Write(b []byte) (int, error) {
7070
func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useStderr bool, params CreateProcessParams) (processid uint32, stdin io.WriteCloser, stdout io.ReadCloser, stderr io.ReadCloser, err error) {
7171

7272
title := "HCSShim::CreateProcessInComputeSystem"
73-
logrus.Debugf(title+"id=%s params=%s", id, params)
73+
logrus.Debugf(title+" id=%s", id)
7474

7575
// Load the DLL and get a handle to the procedure we need
7676
dll, proc, err := loadAndFind(procCreateProcessWithStdHandlesInComputeSystem)
@@ -110,7 +110,7 @@ func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useS
110110
// Get a POINTER to variable to take the pid outparm
111111
pid := new(uint32)
112112

113-
logrus.Debugf(title+" - Calling the procedure itself %s %s", id, paramsJson)
113+
logrus.Debugf(title+" - Calling Win32 %s %s", id, paramsJson)
114114

115115
var stdinHandle, stdoutHandle, stderrHandle syscall.Handle
116116
var stdinParam, stdoutParam, stderrParam uintptr

0 commit comments

Comments
 (0)