Merge pull request #23776 from Microsoft/ShutdownError

Windows: Prevent logging errors when shutting down an already shut down container

hack/vendor.sh

@@ -43,7 +43,7 @@ esac
 
 # the following lines are in sorted order, FYI
 clone git github.com/Azure/go-ansiterm 388960b655244e76e24c75f48631564eaefade62
-clone git github.com/Microsoft/hcsshim v0.3.4
+clone git github.com/Microsoft/hcsshim v0.3.5
 clone git github.com/Microsoft/go-winio v0.3.4
 clone git github.com/Sirupsen/logrus v0.10.0 # logrus is a common dependency among multiple deps
 clone git github.com/docker/libtrust 9cbd2a1374f46905c68a4eb3694a130610adc62a

libcontainerd/container_windows.go

@@ -278,6 +278,8 @@ func (ctr *container) shutdown() error {
 	if err == hcsshim.ErrVmcomputeOperationPending {
 		// Explicit timeout to avoid a (remote) possibility that shutdown hangs indefinitely.
 		err = ctr.hcsContainer.WaitTimeout(shutdownTimeout)
+	} else if err == hcsshim.ErrVmcomputeAlreadyStopped {
+		err = nil
 	}
 
 	if err != nil {
@@ -297,9 +299,12 @@ func (ctr *container) terminate() error {
 
 	if err == hcsshim.ErrVmcomputeOperationPending {
 		err = ctr.hcsContainer.WaitTimeout(terminateTimeout)
+	} else if err == hcsshim.ErrVmcomputeAlreadyStopped {
+		err = nil
 	}
 
 	if err != nil {
+		logrus.Debugf("error terminating container %s %v", ctr.containerID, err)
 		return err
 	}
 

vendor/src/github.com/Microsoft/hcsshim/container.go

@@ -2,8 +2,6 @@ package hcsshim
 
 import (
 	"encoding/json"
-	"errors"
-	"fmt"
 	"runtime"
 	"syscall"
 	"time"
@@ -17,14 +15,6 @@ var (
 
 const pendingUpdatesQuery = `{ "PropertyTypes" : ["PendingUpdates"]}`
 
-// ContainerError is an error encountered in HCS
-type ContainerError struct {
-	Container *container
-	Operation string
-	ExtraInfo string
-	Err       error
-}
-
 type container struct {
 	handle         hcsSystem
 	id             string
@@ -253,7 +243,7 @@ func (container *container) properties(query string) (*containerProperties, erro
 	}
 
 	if propertiesp == nil {
-		return nil, errors.New("Unexpected result from hcsGetComputeSystemProperties, properties should never be nil")
+		return nil, ErrUnexpectedValue
 	}
 	propertiesRaw := convertAndFreeCoTaskMemBytes(propertiesp)
 
@@ -486,46 +476,3 @@ func (container *container) unregisterCallback() error {
 
 	return nil
 }
-
-func (e *ContainerError) Error() string {
-	if e == nil {
-		return "<nil>"
-	}
-
-	if e.Container == nil {
-		return "unexpected nil container for error: " + e.Err.Error()
-	}
-
-	s := "container " + e.Container.id
-
-	if e.Operation != "" {
-		s += " encountered an error during " + e.Operation
-	}
-
-	if e.Err != nil {
-		s += fmt.Sprintf(" failed in Win32: %s (0x%x)", e.Err, win32FromError(e.Err))
-	}
-
-	if e.ExtraInfo != "" {
-		s += " extra info: " + e.ExtraInfo
-	}
-
-	return s
-}
-
-func makeContainerError(container *container, operation string, extraInfo string, err error) error {
-	// Don't wrap errors created in hcsshim
-	if err == ErrTimeout ||
-		err == ErrUnexpectedProcessAbort ||
-		err == ErrUnexpectedContainerExit ||
-		err == ErrHandleClose ||
-		err == ErrInvalidProcessState ||
-		err == ErrInvalidNotificationType ||
-		err == ErrVmcomputeOperationPending {
-		return err
-	}
-
-	containerError := &ContainerError{Container: container, Operation: operation, ExtraInfo: extraInfo, Err: err}
-	logrus.Error(containerError)
-	return containerError
-}

vendor/src/github.com/Microsoft/hcsshim/errors.go

@@ -0,0 +1,149 @@
+package hcsshim
+
+import (
+	"errors"
+	"fmt"
+	"syscall"
+
+	"github.com/Sirupsen/logrus"
+)
+
+var (
+	// ErrHandleClose is an error returned when the handle generating the notification being waited on has been closed
+	ErrHandleClose = errors.New("hcsshim: the handle generating this notification has been closed")
+
+	// ErrInvalidNotificationType is an error encountered when an invalid notification type is used
+	ErrInvalidNotificationType = errors.New("hcsshim: invalid notification type")
+
+	// ErrInvalidProcessState is an error encountered when the process is not in a valid state for the requested operation
+	ErrInvalidProcessState = errors.New("the process is in an invalid state for the attempted operation")
+
+	// ErrTimeout is an error encountered when waiting on a notification times out
+	ErrTimeout = errors.New("hcsshim: timeout waiting for notification")
+
+	// ErrUnexpectedContainerExit is the error returned when a container exits while waiting for
+	// a different expected notification
+	ErrUnexpectedContainerExit = errors.New("unexpected container exit")
+
+	// ErrUnexpectedProcessAbort is the error returned when communication with the compute service
+	// is lost while waiting for a notification
+	ErrUnexpectedProcessAbort = errors.New("lost communication with compute service")
+
+	// ErrUnexpectedValue is an error returned when hcs returns an invalid value
+	ErrUnexpectedValue = errors.New("unexpected value returned from hcs")
+
+	// ErrVmcomputeAlreadyStopped is an error returned when a shutdown or terminate request is made on a stopped container
+	ErrVmcomputeAlreadyStopped = syscall.Errno(0xc0370110)
+
+	// ErrVmcomputeOperationPending is an error returned when the operation is being completed asynchronously
+	ErrVmcomputeOperationPending = syscall.Errno(0xC0370103)
+)
+
+// ProcessError is an error encountered in HCS during an operation on a Process object
+type ProcessError struct {
+	Process   *process
+	Operation string
+	ExtraInfo string
+	Err       error
+}
+
+// ContainerError is an error encountered in HCS during an operation on a Container object
+type ContainerError struct {
+	Container *container
+	Operation string
+	ExtraInfo string
+	Err       error
+}
+
+func isKnownError(err error) bool {
+	// Don't wrap errors created in hcsshim
+	if err == ErrHandleClose ||
+		err == ErrInvalidNotificationType ||
+		err == ErrInvalidProcessState ||
+		err == ErrTimeout ||
+		err == ErrUnexpectedContainerExit ||
+		err == ErrUnexpectedProcessAbort ||
+		err == ErrUnexpectedValue ||
+		err == ErrVmcomputeAlreadyStopped ||
+		err == ErrVmcomputeOperationPending {
+		return true
+	}
+
+	return false
+}
+
+func (e *ContainerError) Error() string {
+	if e == nil {
+		return "<nil>"
+	}
+
+	if e.Container == nil {
+		return "unexpected nil container for error: " + e.Err.Error()
+	}
+
+	s := "container " + e.Container.id
+
+	if e.Operation != "" {
+		s += " encountered an error during " + e.Operation
+	}
+
+	if e.Err != nil {
+		s += fmt.Sprintf(" failed in Win32: %s (0x%x)", e.Err, win32FromError(e.Err))
+	}
+
+	if e.ExtraInfo != "" {
+		s += " extra info: " + e.ExtraInfo
+	}
+
+	return s
+}
+
+func makeContainerError(container *container, operation string, extraInfo string, err error) error {
+	// Return known errors to the client
+	if isKnownError(err) {
+		return err
+	}
+
+	// Log any unexpected errors
+	containerError := &ContainerError{Container: container, Operation: operation, ExtraInfo: extraInfo, Err: err}
+	logrus.Error(containerError)
+	return containerError
+}
+
+func (e *ProcessError) Error() string {
+	if e == nil {
+		return "<nil>"
+	}
+
+	if e.Process == nil {
+		return "Unexpected nil process for error: " + e.Err.Error()
+	}
+
+	s := fmt.Sprintf("process %d", e.Process.processID)
+
+	if e.Process.container != nil {
+		s += " in container " + e.Process.container.id
+	}
+
+	if e.Operation != "" {
+		s += " " + e.Operation
+	}
+
+	if e.Err != nil {
+		s += fmt.Sprintf(" failed in Win32: %s (0x%x)", e.Err, win32FromError(e.Err))
+	}
+
+	return s
+}
+
+func makeProcessError(process *process, operation string, extraInfo string, err error) error {
+	// Return known errors to the client
+	if isKnownError(err) {
+		return err
+	}
+
+	// Log any unexpected errors
+	processError := &ProcessError{Process: process, Operation: operation, ExtraInfo: extraInfo, Err: err}
+	logrus.Error(processError)
+	return processError
+}

vendor/src/github.com/Microsoft/hcsshim/hcsshim.go

@@ -97,8 +97,6 @@ const (
 	ERROR_SHUTDOWN_IN_PROGRESS = syscall.Errno(1115)
 	WSAEINVAL                  = syscall.Errno(10022)
 
-	ErrVmcomputeOperationPending = syscall.Errno(0xC0370103)
-
 	// Timeout on wait calls
 	TimeoutInfinite = 0xFFFFFFFF
 )

vendor/src/github.com/Microsoft/hcsshim/interface.go

@@ -1,33 +1,10 @@
 package hcsshim
 
 import (
-	"errors"
 	"io"
 	"time"
 )
 
-var (
-	// ErrInvalidNotificationType is an error encountered when an invalid notification type is used
-	ErrInvalidNotificationType = errors.New("hcsshim: invalid notification type")
-
-	// ErrTimeout is an error encountered when waiting on a notification times out
-	ErrTimeout = errors.New("hcsshim: timeout waiting for notification")
-
-	// ErrHandleClose is an error returned when the handle generating the notification being waited on has been closed
-	ErrHandleClose = errors.New("hcsshim: the handle generating this notification has been closed")
-
-	// ErrInvalidProcessState is an error encountered when the process is not in a valid state for the requested operation
-	ErrInvalidProcessState = errors.New("the process is in an invalid state for the attempted operation")
-
-	// ErrUnexpectedContainerExit is the error returned when a container exits while waiting for
-	// a different expected notification
-	ErrUnexpectedContainerExit = errors.New("unexpected container exit")
-
-	// ErrUnexpectedProcessAbort is the error returned when communication with the compute service
-	// is lost while waiting for a notification
-	ErrUnexpectedProcessAbort = errors.New("lost communication with compute service")
-)
-
 // ProcessConfig is used as both the input of Container.CreateProcess
 // and to convert the parameters to JSON for passing onto the HCS
 type ProcessConfig struct {

vendor/src/github.com/Microsoft/hcsshim/process.go

@@ -2,8 +2,6 @@ package hcsshim
 
 import (
 	"encoding/json"
-	"errors"
-	"fmt"
 	"io"
 	"syscall"
 	"time"
@@ -11,13 +9,7 @@ import (
 	"github.com/Sirupsen/logrus"
 )
 
-type ProcessError struct {
-	Process   *process
-	Operation string
-	ExtraInfo string
-	Err       error
-}
-
+// ContainerError is an error encountered in HCS
 type process struct {
 	handle         hcsProcess
 	processID      int
@@ -48,7 +40,7 @@ type closeHandle struct {
 }
 
 type processStatus struct {
-	ProcessId      uint32
+	ProcessID      uint32
 	Exited         bool
 	ExitCode       uint32
 	LastWaitResult int32
@@ -223,7 +215,7 @@ func (process *process) properties() (*processStatus, error) {
 	}
 
 	if propertiesp == nil {
-		return nil, errors.New("Unexpected result from hcsGetProcessProperties, properties should never be nil")
+		return nil, ErrUnexpectedValue
 	}
 	propertiesRaw := convertAndFreeCoTaskMemBytes(propertiesp)
 
@@ -396,46 +388,3 @@ func (process *process) unregisterCallback() error {
 
 	return nil
 }
-
-func (e *ProcessError) Error() string {
-	if e == nil {
-		return "<nil>"
-	}
-
-	if e.Process == nil {
-		return "Unexpected nil process for error: " + e.Err.Error()
-	}
-
-	s := fmt.Sprintf("process %d", e.Process.processID)
-
-	if e.Process.container != nil {
-		s += " in container " + e.Process.container.id
-	}
-
-	if e.Operation != "" {
-		s += " " + e.Operation
-	}
-
-	if e.Err != nil {
-		s += fmt.Sprintf(" failed in Win32: %s (0x%x)", e.Err, win32FromError(e.Err))
-	}
-
-	return s
-}
-
-func makeProcessError(process *process, operation string, extraInfo string, err error) error {
-	// Don't wrap errors created in hcsshim
-	if err == ErrTimeout ||
-		err == ErrUnexpectedProcessAbort ||
-		err == ErrUnexpectedContainerExit ||
-		err == ErrHandleClose ||
-		err == ErrInvalidProcessState ||
-		err == ErrInvalidNotificationType ||
-		err == ErrVmcomputeOperationPending {
-		return err
-	}
-
-	processError := &ProcessError{Process: process, Operation: operation, ExtraInfo: extraInfo, Err: err}
-	logrus.Error(processError)
-	return processError
-}