From fadbbd335cfb6d32566267385e820b36c356c14e Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 3 Feb 2016 12:11:21 -0800 Subject: [PATCH 1/2] Windows: Revendor HCSShim@43858ef3 Signed-off-by: John Howard --- hack/vendor.sh | 2 +- .../Microsoft/hcsshim/createprocess.go | 14 +++-- .../github.com/Microsoft/hcsshim/hcsshim.go | 53 ++++++++++--------- .../hcsshim/shutdownterminatecomputesystem.go | 11 ++-- .../Microsoft/hcsshim/waitprocess.go | 9 ++-- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/hack/vendor.sh b/hack/vendor.sh index c6dfc04e9f..36e0ee9810 100755 --- a/hack/vendor.sh +++ b/hack/vendor.sh @@ -16,7 +16,7 @@ clone git github.com/gorilla/mux e444e69cbd clone git github.com/kr/pty 5cf931ef8f clone git github.com/mattn/go-shellwords v1.0.0 clone git github.com/mattn/go-sqlite3 v1.1.0 -clone git github.com/Microsoft/hcsshim 35ad4d808a97203cb1748d7c43167e91f51e7f86 +clone git github.com/Microsoft/hcsshim 43858ef3c5c944dfaaabfbe8b6ea093da7f28dba clone git github.com/mistifyio/go-zfs v2.1.1 clone git github.com/tchap/go-patricia v2.1.0 clone git github.com/vdemeester/shakers 3c10293ce22b900c27acad7b28656196fcc2f73b diff --git a/vendor/src/github.com/Microsoft/hcsshim/createprocess.go b/vendor/src/github.com/Microsoft/hcsshim/createprocess.go index a170a9a1e3..b055ccb664 100644 --- a/vendor/src/github.com/Microsoft/hcsshim/createprocess.go +++ b/vendor/src/github.com/Microsoft/hcsshim/createprocess.go @@ -48,10 +48,9 @@ func makeOpenFiles(hs []syscall.Handle) (_ []io.ReadWriteCloser, err error) { // CreateProcessInComputeSystem starts a process in a container. This is invoked, for example, // as a result of docker run, docker exec, or RUN in Dockerfile. If successful, // it returns the PID of the process. -func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useStderr bool, params CreateProcessParams) (_ uint32, _ io.WriteCloser, _ io.ReadCloser, _ io.ReadCloser, hr uint32, err error) { +func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useStderr bool, params CreateProcessParams) (_ uint32, _ io.WriteCloser, _ io.ReadCloser, _ io.ReadCloser, err error) { title := "HCSShim::CreateProcessInComputeSystem" logrus.Debugf(title+" id=%s", id) - hr = 0xFFFFFFFF // If we are not emulating a console, ignore any console size passed to us if !params.EmulateConsole { @@ -82,14 +81,13 @@ func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useS err = createProcessWithStdHandlesInComputeSystem(id, string(paramsJson), &pid, stdinParam, stdoutParam, stderrParam) if err != nil { - winerr := makeErrorf(err, title, "id=%s params=%v", id, params) - hr = winerr.HResult() + herr := makeErrorf(err, title, "id=%s params=%v", id, params) + err = herr // Windows TP4: Hyper-V Containers may return this error with more than one // concurrent exec. Do not log it as an error - if hr != Win32InvalidArgument { - logrus.Error(winerr) + if herr.Err != WSAEINVAL { + logrus.Error(err) } - err = winerr return } @@ -99,5 +97,5 @@ func CreateProcessInComputeSystem(id string, useStdin bool, useStdout bool, useS } logrus.Debugf(title+" - succeeded id=%s params=%s pid=%d", id, paramsJson, pid) - return pid, pipes[0], pipes[1], pipes[2], 0, nil + return pid, pipes[0], pipes[1], pipes[2], nil } diff --git a/vendor/src/github.com/Microsoft/hcsshim/hcsshim.go b/vendor/src/github.com/Microsoft/hcsshim/hcsshim.go index fc894ca935..339b632ad3 100644 --- a/vendor/src/github.com/Microsoft/hcsshim/hcsshim.go +++ b/vendor/src/github.com/Microsoft/hcsshim/hcsshim.go @@ -43,47 +43,52 @@ const ( // Specific user-visible exit codes WaitErrExecFailed = 32767 - // Known Win32 RC values which should be trapped - Win32PipeHasBeenEnded = 0x8007006d // WaitForProcessInComputeSystem: The pipe has been ended - Win32SystemShutdownIsInProgress = 0x8007045B // ShutdownComputeSystem: A system shutdown is in progress - Win32SpecifiedPathInvalid = 0x800700A1 // ShutdownComputeSystem: The specified path is invalid - Win32SystemCannotFindThePathSpecified = 0x80070003 // ShutdownComputeSystem: The system cannot find the path specified - Win32InvalidArgument = 0x80072726 // CreateProcessInComputeSystem: An invalid argument was supplied - EFail = 0x80004005 + ERROR_GEN_FAILURE = syscall.Errno(31) + ERROR_SHUTDOWN_IN_PROGRESS = syscall.Errno(1115) + WSAEINVAL = syscall.Errno(10022) // Timeout on wait calls TimeoutInfinite = 0xFFFFFFFF ) -type hcsError struct { +type HcsError struct { title string rest string - err error + Err error } -type Win32Error interface { - error - HResult() uint32 +func makeError(err error, title, rest string) *HcsError { + if hr, ok := err.(syscall.Errno); ok { + // Convert the HRESULT to a Win32 error code so that it better matches + // error codes returned from go and other packages. + err = syscall.Errno(win32FromHresult(uint32(hr))) + } + return &HcsError{title, rest, err} } -func makeError(err error, title, rest string) Win32Error { - return &hcsError{title, rest, err} -} - -func makeErrorf(err error, title, format string, a ...interface{}) Win32Error { +func makeErrorf(err error, title, format string, a ...interface{}) *HcsError { return makeError(err, title, fmt.Sprintf(format, a...)) } -func (e *hcsError) HResult() uint32 { - if hr, ok := e.err.(syscall.Errno); ok { - return uint32(hr) - } else { - return EFail +func win32FromError(err error) uint32 { + if herr, ok := err.(*HcsError); ok { + return win32FromError(herr.Err) } + if code, ok := err.(syscall.Errno); ok { + return win32FromHresult(uint32(code)) + } + return uint32(ERROR_GEN_FAILURE) } -func (e *hcsError) Error() string { - return fmt.Sprintf("%s- Win32 API call returned error r1=0x%x err=%s%s", e.title, e.HResult(), e.err, e.rest) +func win32FromHresult(hr uint32) uint32 { + if hr&0x1fff0000 == 0x00070000 { + return hr & 0xffff + } + return hr +} + +func (e *HcsError) Error() string { + return fmt.Sprintf("%s- Win32 API call returned error r1=0x%x err=%s%s", e.title, win32FromError(e.Err), e.Err, e.rest) } func convertAndFreeCoTaskMemString(buffer *uint16) string { diff --git a/vendor/src/github.com/Microsoft/hcsshim/shutdownterminatecomputesystem.go b/vendor/src/github.com/Microsoft/hcsshim/shutdownterminatecomputesystem.go index d006edf77c..27ac734bd8 100644 --- a/vendor/src/github.com/Microsoft/hcsshim/shutdownterminatecomputesystem.go +++ b/vendor/src/github.com/Microsoft/hcsshim/shutdownterminatecomputesystem.go @@ -3,19 +3,19 @@ package hcsshim import "github.com/Sirupsen/logrus" // TerminateComputeSystem force terminates a container. -func TerminateComputeSystem(id string, timeout uint32, context string) (uint32, error) { +func TerminateComputeSystem(id string, timeout uint32, context string) error { return shutdownTerminate(false, id, timeout, context) } // ShutdownComputeSystem shuts down a container by requesting a shutdown within // the container operating system. -func ShutdownComputeSystem(id string, timeout uint32, context string) (uint32, error) { +func ShutdownComputeSystem(id string, timeout uint32, context string) error { return shutdownTerminate(true, id, timeout, context) } // shutdownTerminate is a wrapper for ShutdownComputeSystem and TerminateComputeSystem // which have very similar calling semantics -func shutdownTerminate(shutdown bool, id string, timeout uint32, context string) (uint32, error) { +func shutdownTerminate(shutdown bool, id string, timeout uint32, context string) error { var ( title = "HCSShim::" @@ -35,10 +35,9 @@ func shutdownTerminate(shutdown bool, id string, timeout uint32, context string) } if err != nil { - err := makeErrorf(err, title, "id=%s context=%s", id, context) - return err.HResult(), err + return makeErrorf(err, title, "id=%s context=%s", id, context) } logrus.Debugf(title+" succeeded id=%s context=%s", id, context) - return 0, nil + return nil } diff --git a/vendor/src/github.com/Microsoft/hcsshim/waitprocess.go b/vendor/src/github.com/Microsoft/hcsshim/waitprocess.go index c9f4617711..e916140399 100644 --- a/vendor/src/github.com/Microsoft/hcsshim/waitprocess.go +++ b/vendor/src/github.com/Microsoft/hcsshim/waitprocess.go @@ -3,8 +3,8 @@ package hcsshim import "github.com/Sirupsen/logrus" // WaitForProcessInComputeSystem waits for a process ID to terminate and returns -// the exit code. Returns exitcode, errno, error -func WaitForProcessInComputeSystem(id string, processid uint32, timeout uint32) (int32, uint32, error) { +// the exit code. Returns exitcode, error +func WaitForProcessInComputeSystem(id string, processid uint32, timeout uint32) (int32, error) { title := "HCSShim::WaitForProcessInComputeSystem" logrus.Debugf(title+" id=%s processid=%d", id, processid) @@ -12,10 +12,9 @@ func WaitForProcessInComputeSystem(id string, processid uint32, timeout uint32) var exitCode uint32 err := waitForProcessInComputeSystem(id, processid, timeout, &exitCode) if err != nil { - err := makeErrorf(err, title, "id=%s", id) - return 0, err.HResult(), err + return 0, makeErrorf(err, title, "id=%s", id) } logrus.Debugf(title+" succeeded id=%s processid=%d exitcode=%d", id, processid, exitCode) - return int32(exitCode), 0, nil + return int32(exitCode), nil } From 54263a93933a4a53f44ddea58ac16b2526e136e3 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 3 Feb 2016 13:04:41 -0800 Subject: [PATCH 2/2] Windows: Use new error code mechanism from HCS Signed-off-by: John Howard --- daemon/execdriver/windows/exec.go | 14 +++--- daemon/execdriver/windows/run.go | 58 +++++++++++++--------- daemon/execdriver/windows/terminatekill.go | 8 +-- 3 files changed, 45 insertions(+), 35 deletions(-) diff --git a/daemon/execdriver/windows/exec.go b/daemon/execdriver/windows/exec.go index 1b6c06bd14..c9129a8bbd 100644 --- a/daemon/execdriver/windows/exec.go +++ b/daemon/execdriver/windows/exec.go @@ -4,6 +4,7 @@ package windows import ( "fmt" + "syscall" "github.com/Microsoft/hcsshim" "github.com/Sirupsen/logrus" @@ -17,7 +18,6 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo term execdriver.Terminal err error exitCode int32 - errno uint32 ) active := d.activeContainers[c.ID] @@ -41,13 +41,13 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo } // Start the command running in the container. - pid, stdin, stdout, stderr, rc, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !processConfig.Tty, createProcessParms) + pid, stdin, stdout, stderr, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !processConfig.Tty, createProcessParms) if err != nil { // TODO Windows: TP4 Workaround. In Hyper-V containers, there is a limitation // of one exec per container. This should be fixed post TP4. CreateProcessInComputeSystem // will return a specific error which we handle here to give a good error message // back to the user instead of an inactionable "An invalid argument was supplied" - if rc == hcsshim.Win32InvalidArgument { + if herr, ok := err.(*hcsshim.HcsError); ok && herr.Err == hcsshim.WSAEINVAL { return -1, fmt.Errorf("The limit of docker execs per Hyper-V container has been exceeded") } logrus.Errorf("CreateProcessInComputeSystem() failed %s", err) @@ -75,12 +75,12 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo hooks.Start(&c.ProcessConfig, int(pid), chOOM) } - if exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite); err != nil { - if errno == hcsshim.Win32PipeHasBeenEnded { - logrus.Debugf("Exiting Run() after WaitForProcessInComputeSystem failed with recognised error 0x%X", errno) + if exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite); err != nil { + if herr, ok := err.(*hcsshim.HcsError); ok && herr.Err == syscall.ERROR_BROKEN_PIPE { + logrus.Debugf("Exiting Run() after WaitForProcessInComputeSystem failed with recognised error %s", err) return hcsshim.WaitErrExecFailed, nil } - logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): 0x%X %s", errno, err) + logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err) return -1, err } diff --git a/daemon/execdriver/windows/run.go b/daemon/execdriver/windows/run.go index 1d720e4661..4bc484af9d 100644 --- a/daemon/execdriver/windows/run.go +++ b/daemon/execdriver/windows/run.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "time" "github.com/Microsoft/hcsshim" @@ -20,6 +21,16 @@ import ( // preconfigured on the server. const defaultContainerNAT = "ContainerNAT" +// Win32 error codes that are used for various workarounds +// These really should be ALL_CAPS to match golangs syscall library and standard +// Win32 error conventions, but golint insists on CamelCase. +const ( + CoEClassstring = syscall.Errno(0x800401F3) // Invalid class string + ErrorNoNetwork = syscall.Errno(1222) // The network is not present or not started + ErrorBadPathname = syscall.Errno(161) // The specified path is invalid + ErrorInvalidObject = syscall.Errno(0x800710D8) // The object identifier does not represent a valid object +) + type layer struct { ID string Path string @@ -237,17 +248,19 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd err = hcsshim.CreateComputeSystem(c.ID, configuration) if err != nil { if TP4RetryHack { - if !strings.Contains(err.Error(), `Win32 API call returned error r1=0x800401f3`) && // Invalid class string - !strings.Contains(err.Error(), `Win32 API call returned error r1=0x80070490`) && // Element not found - !strings.Contains(err.Error(), `Win32 API call returned error r1=0x80070002`) && // The system cannot find the file specified - !strings.Contains(err.Error(), `Win32 API call returned error r1=0x800704c6`) && // The network is not present or not started - !strings.Contains(err.Error(), `Win32 API call returned error r1=0x800700a1`) && // The specified path is invalid - !strings.Contains(err.Error(), `Win32 API call returned error r1=0x800710d8`) { // The object identifier does not represent a valid object - logrus.Debugln("Failed to create temporary container ", err) - return execdriver.ExitStatus{ExitCode: -1}, err + if herr, ok := err.(*hcsshim.HcsError); ok { + if herr.Err != syscall.ERROR_NOT_FOUND && // Element not found + herr.Err != syscall.ERROR_FILE_NOT_FOUND && // The system cannot find the file specified + herr.Err != ErrorNoNetwork && // The network is not present or not started + herr.Err != ErrorBadPathname && // The specified path is invalid + herr.Err != CoEClassstring && // Invalid class string + herr.Err != ErrorInvalidObject { // The object identifier does not represent a valid object + logrus.Debugln("Failed to create temporary container ", err) + return execdriver.ExitStatus{ExitCode: -1}, err + } + logrus.Warnf("Invoking Windows TP4 retry hack (%d of %d)", i, maxAttempts-1) + time.Sleep(50 * time.Millisecond) } - logrus.Warnf("Invoking Windows TP4 retry hack (%d of %d)", i, maxAttempts-1) - time.Sleep(50 * time.Millisecond) } } else { break @@ -265,16 +278,17 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd // Stop the container if forceKill { logrus.Debugf("Forcibly terminating container %s", c.ID) - if errno, err := hcsshim.TerminateComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil { - logrus.Warnf("Ignoring error from TerminateComputeSystem 0x%X %s", errno, err) + if err := hcsshim.TerminateComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil { + logrus.Warnf("Ignoring error from TerminateComputeSystem %s", err) } } else { logrus.Debugf("Shutting down container %s", c.ID) - if errno, err := hcsshim.ShutdownComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil { - if errno != hcsshim.Win32SystemShutdownIsInProgress && - errno != hcsshim.Win32SpecifiedPathInvalid && - errno != hcsshim.Win32SystemCannotFindThePathSpecified { - logrus.Warnf("Ignoring error from ShutdownComputeSystem 0x%X %s", errno, err) + if err := hcsshim.ShutdownComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil { + if herr, ok := err.(*hcsshim.HcsError); !ok || + (herr.Err != hcsshim.ERROR_SHUTDOWN_IN_PROGRESS && + herr.Err != ErrorBadPathname && + herr.Err != syscall.ERROR_PATH_NOT_FOUND) { + logrus.Warnf("Ignoring error from ShutdownComputeSystem %s", err) } } } @@ -296,7 +310,7 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd } // Start the command running in the container. - pid, stdin, stdout, stderr, _, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !c.ProcessConfig.Tty, createProcessParms) + pid, stdin, stdout, stderr, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !c.ProcessConfig.Tty, createProcessParms) if err != nil { logrus.Errorf("CreateProcessInComputeSystem() failed %s", err) return execdriver.ExitStatus{ExitCode: -1}, err @@ -333,13 +347,9 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd hooks.Start(&c.ProcessConfig, int(pid), chOOM) } - var ( - exitCode int32 - errno uint32 - ) - exitCode, errno, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite) + exitCode, err := hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite) if err != nil { - if errno != hcsshim.Win32PipeHasBeenEnded { + if herr, ok := err.(*hcsshim.HcsError); ok && herr.Err != syscall.ERROR_BROKEN_PIPE { logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err) } // Do NOT return err here as the container would have diff --git a/daemon/execdriver/windows/terminatekill.go b/daemon/execdriver/windows/terminatekill.go index 7068a68fa9..d20b0e2b9d 100644 --- a/daemon/execdriver/windows/terminatekill.go +++ b/daemon/execdriver/windows/terminatekill.go @@ -28,8 +28,8 @@ func kill(id string, pid int, sig syscall.Signal) error { if sig == syscall.SIGKILL || forceKill { // Terminate the compute system - if errno, err := hcsshim.TerminateComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil { - logrus.Errorf("Failed to terminate %s - 0x%X %q", id, errno, err) + if err := hcsshim.TerminateComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil { + logrus.Errorf("Failed to terminate %s - %q", id, err) } } else { @@ -41,8 +41,8 @@ func kill(id string, pid int, sig syscall.Signal) error { } // Shutdown the compute system - if errno, err := hcsshim.ShutdownComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil { - logrus.Errorf("Failed to shutdown %s - 0x%X %q", id, errno, err) + if err := hcsshim.ShutdownComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil { + logrus.Errorf("Failed to shutdown %s - %q", id, err) } } return err