runc delete: do not ignore error from destroy

If container.Destroy() has failed, runc destroy still return 0, which is
wrong and can result in other issues down the line.

Let's always return error from destroy in runc delete.

For runc checkpoint and runc run, we still treat it as a warning.

Co-authored-by: Zhang Tianyang <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

checkpoint.go

@@ -70,7 +70,9 @@ checkpointed.`,
 		err = container.Checkpoint(options)
 		if err == nil && !(options.LeaveRunning || options.PreDump) {
 			// Destroy the container unless we tell CRIU to keep it.
-			destroy(container)
+			if err := container.Destroy(); err != nil {
+				logrus.Warn(err)
+			}
 		}
 		return err
 	},

delete.go

@@ -18,8 +18,7 @@ func killContainer(container *libcontainer.Container) error {
 	for i := 0; i < 100; i++ {
 		time.Sleep(100 * time.Millisecond)
 		if err := container.Signal(unix.Signal(0)); err != nil {
-			destroy(container)
-			return nil
+			return container.Destroy()
 		}
 	}
 	return errors.New("container init still running")
@@ -80,13 +79,11 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
 		}
 		switch s {
 		case libcontainer.Stopped:
-			destroy(container)
+			return container.Destroy()
 		case libcontainer.Created:
 			return killContainer(container)
 		default:
 			return fmt.Errorf("cannot delete container %s that is not stopped: %s", id, s)
 		}
-
-		return nil
 	},
 }

libcontainer/container_linux.go

@@ -874,7 +874,10 @@ func (c *Container) newInitConfig(process *Process) *initConfig {
 func (c *Container) Destroy() error {
 	c.m.Lock()
 	defer c.m.Unlock()
-	return c.state.destroy()
+	if err := c.state.destroy(); err != nil {
+		return fmt.Errorf("unable to destroy container: %w", err)
+	}
+	return nil
 }
 
 // Pause pauses the container, if its state is RUNNING or CREATED, changing

utils_linux.go

@@ -88,12 +88,6 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
 	return lp, nil
 }
 
-func destroy(container *libcontainer.Container) {
-	if err := container.Destroy(); err != nil {
-		logrus.Error(err)
-	}
-}
-
 // setupIO modifies the given process config according to the options.
 func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, detach bool, sockpath string) (*tty, error) {
 	if createTTY {
@@ -303,7 +297,9 @@ func (r *runner) run(config *specs.Process) (int, error) {
 
 func (r *runner) destroy() {
 	if r.shouldDestroy {
-		destroy(r.container)
+		if err := r.container.Destroy(); err != nil {
+			logrus.Warn(err)
+		}
 	}
 }