Browse code

Merge pull request #22989 from Microsoft/StartCleanup

Windows: Adding missing cleanup call when container start fails

Vincent Demeester authored on 2016/06/01 22:42:47
Showing 2 changed files
... ...
@@ -153,7 +153,7 @@ func (clnt *client) Create(containerID string, spec Spec, options ...CreateOptio
153 153
 	}
154 154
 
155 155
 	// Call start, and if it fails, delete the container from our
156
-	// internal structure, and also keep HCS in sync by deleting the
156
+	// internal structure, start will keep HCS in sync by deleting the
157 157
 	// container there.
158 158
 	logrus.Debugf("Create() id=%s, Calling start()", containerID)
159 159
 	if err := container.start(); err != nil {
... ...
@@ -42,13 +42,18 @@ func (ctr *container) start() error {
42 42
 	// until the container is done with the servicing execution.
43 43
 	logrus.Debugln("Starting container ", ctr.containerID)
44 44
 	if err = ctr.hcsContainer.Start(); err != nil {
45
-		logrus.Errorf("Failed to start compute system: %s", err)
45
+		logrus.Errorf("Failed to start container: %s", err)
46
+		if err := ctr.terminate(); err != nil {
47
+			logrus.Errorf("Failed to cleanup after a failed Start. %s", err)
48
+		} else {
49
+			logrus.Debugln("Cleaned up after failed Start by calling Terminate")
50
+		}
46 51
 		return err
47 52
 	}
48 53
 
49 54
 	for _, option := range ctr.options {
50 55
 		if s, ok := option.(*ServicingOption); ok && s.IsServicing {
51
-			// Since the servicing operation is complete when StartCommputeSystem returns without error,
56
+			// Since the servicing operation is complete when Start returns without error,
52 57
 			// we can shutdown (which triggers merge) and exit early.
53 58
 			return ctr.shutdown()
54 59
 		}
... ...
@@ -76,8 +81,8 @@ func (ctr *container) start() error {
76 76
 	hcsProcess, err := ctr.hcsContainer.CreateProcess(createProcessParms)
77 77
 	if err != nil {
78 78
 		logrus.Errorf("CreateProcess() failed %s", err)
79
-		if err2 := ctr.terminate(); err2 != nil {
80
-			logrus.Debugf("Failed to cleanup after a failed CreateProcess. Ignoring this. %s", err2)
79
+		if err := ctr.terminate(); err != nil {
80
+			logrus.Errorf("Failed to cleanup after a failed CreateProcess. %s", err)
81 81
 		} else {
82 82
 			logrus.Debugln("Cleaned up after failed CreateProcess by calling Terminate")
83 83
 		}
... ...
@@ -96,7 +101,7 @@ func (ctr *container) start() error {
96 96
 	if err != nil {
97 97
 		logrus.Errorf("failed to get stdio pipes: %s", err)
98 98
 		if err := ctr.terminate(); err != nil {
99
-			logrus.Debugf("Failed to cleanup after a failed CreateProcess. Ignoring this. %s", err)
99
+			logrus.Errorf("Failed to cleanup after a failed Stdio. %s", err)
100 100
 		}
101 101
 		return err
102 102
 	}
... ...
@@ -150,7 +155,7 @@ func (ctr *container) waitExit(process *process, isFirstProcessToStart bool) err
150 150
 	err := process.hcsProcess.Wait()
151 151
 	if err != nil {
152 152
 		if herr, ok := err.(*hcsshim.ProcessError); ok && herr.Err != syscall.ERROR_BROKEN_PIPE {
153
-			logrus.Warnf("WaitForProcessInComputeSystem failed (container may have been killed): %s", err)
153
+			logrus.Warnf("Wait failed (container may have been killed): %s", err)
154 154
 		}
155 155
 		// Fall through here, do not return. This ensures we attempt to continue the
156 156
 		// shutdown in HCS and tell the docker engine that the process/container