Merge pull request #20210 from Microsoft/jjh/hcswin32-v2

Windows: Revendor HCS to use revised error scheme

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 exitCode, err = hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite); err != nil {
-		if errno == hcsshim.Win32PipeHasBeenEnded {
+		if herr, ok := err.(*hcsshim.HcsError); ok && herr.Err == syscall.ERROR_BROKEN_PIPE {
-			logrus.Debugf("Exiting Run() after WaitForProcessInComputeSystem failed with recognised error 0x%X", errno)
+			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
 	}
 

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,18 +248,20 @@ 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
+				if herr, ok := err.(*hcsshim.HcsError); ok {
-					!strings.Contains(err.Error(), `Win32 API call returned error r1=0x80070490`) && // Element not found
+					if herr.Err != syscall.ERROR_NOT_FOUND && // Element not found
-					!strings.Contains(err.Error(), `Win32 API call returned error r1=0x80070002`) && // The system cannot find the file specified
+						herr.Err != syscall.ERROR_FILE_NOT_FOUND && // 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
+						herr.Err != ErrorNoNetwork && // The network is not present or not started
-					!strings.Contains(err.Error(), `Win32 API call returned error r1=0x800700a1`) && // The specified path is invalid
+						herr.Err != ErrorBadPathname && // 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
+						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)
 				}
+			}
 		} 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 {
+			if err := hcsshim.TerminateComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
-				logrus.Warnf("Ignoring error from TerminateComputeSystem 0x%X %s", errno, err)
+				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 err := hcsshim.ShutdownComputeSystem(c.ID, hcsshim.TimeoutInfinite, "exec-run-defer"); err != nil {
-				if errno != hcsshim.Win32SystemShutdownIsInProgress &&
+				if herr, ok := err.(*hcsshim.HcsError); !ok ||
-					errno != hcsshim.Win32SpecifiedPathInvalid &&
+					(herr.Err != hcsshim.ERROR_SHUTDOWN_IN_PROGRESS &&
-					errno != hcsshim.Win32SystemCannotFindThePathSpecified {
+						herr.Err != ErrorBadPathname &&
-					logrus.Warnf("Ignoring error from ShutdownComputeSystem 0x%X %s", errno, err)
+						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, err := hcsshim.WaitForProcessInComputeSystem(c.ID, pid, hcsshim.TimeoutInfinite)
-		exitCode int32
-		errno    uint32
-	)
-	exitCode, errno, 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

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 {
+		if err := hcsshim.TerminateComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
-			logrus.Errorf("Failed to terminate %s - 0x%X %q", id, errno, err)
+			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 {
+		if err := hcsshim.ShutdownComputeSystem(id, hcsshim.TimeoutInfinite, context); err != nil {
-			logrus.Errorf("Failed to shutdown %s - 0x%X %q", id, errno, err)
+			logrus.Errorf("Failed to shutdown %s - %q", id, err)
 		}
 	}
 	return err

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

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)
+		herr := makeErrorf(err, title, "id=%s params=%v", id, params)
-		hr = winerr.HResult()
+		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 {
+		if herr.Err != WSAEINVAL {
-			logrus.Error(winerr)
+			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
 }

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
+	ERROR_GEN_FAILURE          = syscall.Errno(31)
-	Win32PipeHasBeenEnded                 = 0x8007006d // WaitForProcessInComputeSystem: The pipe has been ended
+	ERROR_SHUTDOWN_IN_PROGRESS = syscall.Errno(1115)
-	Win32SystemShutdownIsInProgress       = 0x8007045B // ShutdownComputeSystem: A system shutdown is in progress
+	WSAEINVAL                  = syscall.Errno(10022)
-	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
 
 	// Timeout on wait calls
 	TimeoutInfinite = 0xFFFFFFFF
 )
 
-type hcsError struct {
+type HcsError struct {
 	title string
 	rest  string
-	err   error
+	Err   error
 }
 
-type Win32Error interface {
+func makeError(err error, title, rest string) *HcsError {
-	error
+	if hr, ok := err.(syscall.Errno); ok {
-	HResult() uint32
+		// 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 {
+func makeErrorf(err error, title, format string, a ...interface{}) *HcsError {
-	return &hcsError{title, rest, err}
-}
-
-func makeErrorf(err error, title, format string, a ...interface{}) Win32Error {
 	return makeError(err, title, fmt.Sprintf(format, a...))
 }
 
-func (e *hcsError) HResult() uint32 {
+func win32FromError(err error) uint32 {
-	if hr, ok := e.err.(syscall.Errno); ok {
+	if herr, ok := err.(*HcsError); ok {
-		return uint32(hr)
+		return win32FromError(herr.Err)
-	} else {
-		return EFail
 	}
+	if code, ok := err.(syscall.Errno); ok {
+		return win32FromHresult(uint32(code))
+	}
+	return uint32(ERROR_GEN_FAILURE)
 }
 
-func (e *hcsError) Error() string {
+func win32FromHresult(hr uint32) uint32 {
-	return fmt.Sprintf("%s- Win32 API call returned error r1=0x%x err=%s%s", e.title, e.HResult(), e.err, e.rest)
+	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 {

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 makeErrorf(err, title, "id=%s context=%s", id, context)
-		return err.HResult(), err
 	}
 
 	logrus.Debugf(title+" succeeded id=%s context=%s", id, context)
-	return 0, nil
+	return nil
 }

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
+// the exit code. Returns exitcode, error
-func WaitForProcessInComputeSystem(id string, processid uint32, timeout uint32) (int32, uint32, 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, makeErrorf(err, title, "id=%s", id)
-		return 0, err.HResult(), err
 	}
 
 	logrus.Debugf(title+" succeeded id=%s processid=%d exitcode=%d", id, processid, exitCode)
-	return int32(exitCode), 0, nil
+	return int32(exitCode), nil
 }