Updating call sequence for servicing Windows containers

This change adjusts the calling pattern for servcing containers to use waiting on the process instead of expecting start to block.  This is safer, as it avoids timeouts in the start code path for the potentially expensive update operation.

Signed-off-by: Stefan J. Wernli <swernli@microsoft.com>

libcontainerd/container_windows.go

@@ -37,6 +37,13 @@ func (ctr *container) newProcess(friendlyName string) *process {
 
 func (ctr *container) start() error {
 	var err error
+	isServicing := false
+
+	for _, option := range ctr.options {
+		if s, ok := option.(*ServicingOption); ok && s.IsServicing {
+			isServicing = true
+		}
+	}
 
 	// Start the container.  If this is a servicing container, this call will block
 	// until the container is done with the servicing execution.
@@ -51,14 +58,6 @@ func (ctr *container) start() error {
 		return err
 	}
 
-	for _, option := range ctr.options {
-		if s, ok := option.(*ServicingOption); ok && s.IsServicing {
-			// Since the servicing operation is complete when Start returns without error,
-			// we can shutdown (which triggers merge) and exit early.
-			return ctr.shutdown()
-		}
-	}
-
 	// Note we always tell HCS to
 	// create stdout as it's required regardless of '-i' or '-t' options, so that
 	// docker can always grab the output through logs. We also tell HCS to always
@@ -68,9 +67,9 @@ func (ctr *container) start() error {
 		EmulateConsole:   ctr.ociSpec.Process.Terminal,
 		WorkingDirectory: ctr.ociSpec.Process.Cwd,
 		ConsoleSize:      ctr.ociSpec.Process.InitialConsoleSize,
-		CreateStdInPipe:  true,
-		CreateStdOutPipe: true,
-		CreateStdErrPipe: !ctr.ociSpec.Process.Terminal,
+		CreateStdInPipe:  !isServicing,
+		CreateStdOutPipe: !isServicing,
+		CreateStdErrPipe: !ctr.ociSpec.Process.Terminal && !isServicing,
 	}
 
 	// Configure the environment for the process
@@ -95,6 +94,19 @@ func (ctr *container) start() error {
 	pid := hcsProcess.Pid()
 	ctr.process.hcsProcess = hcsProcess
 
+	// If this is a servicing container, wait on the process synchronously here and
+	// immediately call shutdown/terminate when it returns.
+	if isServicing {
+		exitCode := ctr.waitProcessExitCode(&ctr.process)
+
+		if exitCode != 0 {
+			logrus.Warnf("Servicing container %s returned non-zero exit code %d", ctr.containerID, exitCode)
+			return ctr.terminate()
+		}
+
+		return ctr.shutdown()
+	}
+
 	var stdout, stderr io.ReadCloser
 	var stdin io.WriteCloser
 	stdin, stdout, stderr, err = hcsProcess.Stdio()
@@ -145,12 +157,8 @@ func (ctr *container) start() error {
 
 }
 
-// waitExit runs as a goroutine waiting for the process to exit. It's
-// equivalent to (in the linux containerd world) where events come in for
-// state change notifications from containerd.
-func (ctr *container) waitExit(process *process, isFirstProcessToStart bool) error {
-	logrus.Debugln("waitExit on pid", process.systemPid)
-
+// waitProcessExitCode will wait for the given process to exit and return its error code.
+func (ctr *container) waitProcessExitCode(process *process) int {
 	// Block indefinitely for the process to exit.
 	err := process.hcsProcess.Wait()
 	if err != nil {
@@ -176,6 +184,17 @@ func (ctr *container) waitExit(process *process, isFirstProcessToStart bool) err
 		logrus.Error(err)
 	}
 
+	return exitCode
+}
+
+// waitExit runs as a goroutine waiting for the process to exit. It's
+// equivalent to (in the linux containerd world) where events come in for
+// state change notifications from containerd.
+func (ctr *container) waitExit(process *process, isFirstProcessToStart bool) error {
+	logrus.Debugln("waitExit on pid", process.systemPid)
+
+	exitCode := ctr.waitProcessExitCode(process)
+
 	// Assume the container has exited
 	si := StateInfo{
 		CommonStateInfo: CommonStateInfo{