From f2ad7be2c4aa13413d539887e8c13fb47bea7254 Mon Sep 17 00:00:00 2001
From: "Stefan J. Wernli" <swernli@microsoft.com>
Date: Wed, 8 Jun 2016 18:57:25 -0700
Subject: [PATCH] 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 | 53 ++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/libcontainerd/container_windows.go b/libcontainerd/container_windows.go
index 152a7a2814..7e36e857ec 100644
--- a/libcontainerd/container_windows.go
+++ b/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{