Ensure we can always terminate the parent process on error

As we all know, we should terminate the parent process if there is an
error when starting the container process, but these terminate function
are called in many places, for example: `initProcess`, `setnsProcess`,
and `Container`, if we forget this terminate call for some errors, it
will let the container in unknown state, so we should change to call it
in some final places.

Signed-off-by: lifubang <[email protected]>

Author: lifubang

libcontainer/container_linux.go

@@ -201,18 +201,28 @@ func (c *Container) Set(config configs.Config) error {
 
 // Start starts a process inside the container. Returns error if process fails
 // to start. You can track process lifecycle with passed Process structure.
-func (c *Container) Start(process *Process) error {
+func (c *Container) Start(process *Process) (retErr error) {
 	c.m.Lock()
-	defer c.m.Unlock()
+	defer func() {
+		if retErr != nil {
+			c.terminate(process)
+		}
+		c.m.Unlock()
+	}()
 	return c.start(process)
 }
 
 // Run immediately starts the process inside the container. Returns an error if
 // the process fails to start. It does not block waiting for the exec fifo
 // after start returns but opens the fifo after start returns.
-func (c *Container) Run(process *Process) error {
+func (c *Container) Run(process *Process) (retErr error) {
 	c.m.Lock()
-	defer c.m.Unlock()
+	defer func() {
+		if retErr != nil {
+			c.terminate(process)
+		}
+		c.m.Unlock()
+	}()
 	if err := c.start(process); err != nil {
 		return err
 	}
@@ -229,6 +239,30 @@ func (c *Container) Exec() error {
 	return c.exec()
 }
 
+// terminate is to kill the container's init/exec process when got failure.
+func (c *Container) terminate(process *Process) {
+	if process.ops == nil {
+		return
+	}
+	if process.Init {
+		if err := ignoreTerminateErrors(process.ops.terminate()); err != nil {
+			logrus.WithError(err).Warn("unable to terminate initProcess")
+		}
+		if err := c.cgroupManager.Destroy(); err != nil {
+			logrus.WithError(err).Warn("unable to destroy cgroupManager")
+		}
+		if c.intelRdtManager != nil {
+			if err := c.intelRdtManager.Destroy(); err != nil {
+				logrus.WithError(err).Warn("unable to destroy intelRdtManager")
+			}
+		}
+		return
+	}
+	if err := ignoreTerminateErrors(process.ops.terminate()); err != nil {
+		logrus.WithError(err).Warn("unable to terminate setnsProcess")
+	}
+}
+
 func (c *Container) exec() error {
 	path := filepath.Join(c.stateDir, execFifoFilename)
 	pid := c.initProcess.pid()
@@ -359,12 +393,7 @@ func (c *Container) start(process *Process) (retErr error) {
 				return err
 			}
 
-			if err := c.config.Hooks.Run(configs.Poststart, s); err != nil {
-				if err := ignoreTerminateErrors(parent.terminate()); err != nil {
-					logrus.Warn(fmt.Errorf("error running poststart hook: %w", err))
-				}
-				return err
-			}
+			return c.config.Hooks.Run(configs.Poststart, s)
 		}
 	}
 	return nil

libcontainer/process.go

@@ -12,6 +12,7 @@ import (
 var errInvalidProcess = errors.New("invalid process")
 
 type processOperations interface {
+	terminate() error
 	wait() (*os.ProcessState, error)
 	signal(sig os.Signal) error
 	pid() int

libcontainer/process_linux.go

@@ -151,10 +151,6 @@ func (p *setnsProcess) start() (retErr error) {
 			if werr != nil {
 				logrus.WithError(werr).Warn()
 			}
-			err := ignoreTerminateErrors(p.terminate())
-			if err != nil {
-				logrus.WithError(err).Warn("unable to terminate setnsProcess")
-			}
 		}
 	}()
 
@@ -563,16 +559,6 @@ func (p *initProcess) start() (retErr error) {
 			if werr != nil {
 				logrus.WithError(werr).Warn()
 			}
-
-			// Terminate the process to ensure we can remove cgroups.
-			if err := ignoreTerminateErrors(p.terminate()); err != nil {
-				logrus.WithError(err).Warn("unable to terminate initProcess")
-			}
-
-			_ = p.manager.Destroy()
-			if p.intelRdtManager != nil {
-				_ = p.intelRdtManager.Destroy()
-			}
 		}
 	}()