Browse Source

Windows: Adding missing cleanup call when container start fails

Signed-off-by: Darren Stahl <darst@microsoft.com>
Darren Stahl 9 years ago
parent
commit
054992e291
2 changed files with 12 additions and 7 deletions
  1. 1 1
      libcontainerd/client_windows.go
  2. 11 6
      libcontainerd/container_windows.go

+ 1 - 1
libcontainerd/client_windows.go

@@ -153,7 +153,7 @@ func (clnt *client) Create(containerID string, spec Spec, options ...CreateOptio
 	}
 	}
 
 
 	// Call start, and if it fails, delete the container from our
 	// Call start, and if it fails, delete the container from our
-	// internal structure, and also keep HCS in sync by deleting the
+	// internal structure, start will keep HCS in sync by deleting the
 	// container there.
 	// container there.
 	logrus.Debugf("Create() id=%s, Calling start()", containerID)
 	logrus.Debugf("Create() id=%s, Calling start()", containerID)
 	if err := container.start(); err != nil {
 	if err := container.start(); err != nil {

+ 11 - 6
libcontainerd/container_windows.go

@@ -42,13 +42,18 @@ func (ctr *container) start() error {
 	// until the container is done with the servicing execution.
 	// until the container is done with the servicing execution.
 	logrus.Debugln("Starting container ", ctr.containerID)
 	logrus.Debugln("Starting container ", ctr.containerID)
 	if err = ctr.hcsContainer.Start(); err != nil {
 	if err = ctr.hcsContainer.Start(); err != nil {
-		logrus.Errorf("Failed to start compute system: %s", err)
+		logrus.Errorf("Failed to start container: %s", err)
+		if err := ctr.terminate(); err != nil {
+			logrus.Errorf("Failed to cleanup after a failed Start. %s", err)
+		} else {
+			logrus.Debugln("Cleaned up after failed Start by calling Terminate")
+		}
 		return err
 		return err
 	}
 	}
 
 
 	for _, option := range ctr.options {
 	for _, option := range ctr.options {
 		if s, ok := option.(*ServicingOption); ok && s.IsServicing {
 		if s, ok := option.(*ServicingOption); ok && s.IsServicing {
-			// Since the servicing operation is complete when StartCommputeSystem returns without error,
+			// Since the servicing operation is complete when Start returns without error,
 			// we can shutdown (which triggers merge) and exit early.
 			// we can shutdown (which triggers merge) and exit early.
 			return ctr.shutdown()
 			return ctr.shutdown()
 		}
 		}
@@ -76,8 +81,8 @@ func (ctr *container) start() error {
 	hcsProcess, err := ctr.hcsContainer.CreateProcess(createProcessParms)
 	hcsProcess, err := ctr.hcsContainer.CreateProcess(createProcessParms)
 	if err != nil {
 	if err != nil {
 		logrus.Errorf("CreateProcess() failed %s", err)
 		logrus.Errorf("CreateProcess() failed %s", err)
-		if err2 := ctr.terminate(); err2 != nil {
-			logrus.Debugf("Failed to cleanup after a failed CreateProcess. Ignoring this. %s", err2)
+		if err := ctr.terminate(); err != nil {
+			logrus.Errorf("Failed to cleanup after a failed CreateProcess. %s", err)
 		} else {
 		} else {
 			logrus.Debugln("Cleaned up after failed CreateProcess by calling Terminate")
 			logrus.Debugln("Cleaned up after failed CreateProcess by calling Terminate")
 		}
 		}
@@ -96,7 +101,7 @@ func (ctr *container) start() error {
 	if err != nil {
 	if err != nil {
 		logrus.Errorf("failed to get stdio pipes: %s", err)
 		logrus.Errorf("failed to get stdio pipes: %s", err)
 		if err := ctr.terminate(); err != nil {
 		if err := ctr.terminate(); err != nil {
-			logrus.Debugf("Failed to cleanup after a failed CreateProcess. Ignoring this. %s", err)
+			logrus.Errorf("Failed to cleanup after a failed Stdio. %s", err)
 		}
 		}
 		return err
 		return err
 	}
 	}
@@ -150,7 +155,7 @@ func (ctr *container) waitExit(process *process, isFirstProcessToStart bool) err
 	err := process.hcsProcess.Wait()
 	err := process.hcsProcess.Wait()
 	if err != nil {
 	if err != nil {
 		if herr, ok := err.(*hcsshim.ProcessError); ok && herr.Err != syscall.ERROR_BROKEN_PIPE {
 		if herr, ok := err.(*hcsshim.ProcessError); ok && herr.Err != syscall.ERROR_BROKEN_PIPE {
-			logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err)
+			logrus.Warnf("Wait failed (container may have been killed): %s", err)
 		}
 		}
 		// Fall through here, do not return. This ensures we attempt to continue the
 		// Fall through here, do not return. This ensures we attempt to continue the
 		// shutdown in HCS and tell the docker engine that the process/container
 		// shutdown in HCS and tell the docker engine that the process/container