merge branch 'pr-3214'

Kir Kolyshkin (3):
  create, run: amend final errors
  startContainer: minor refactor
  delete, start: remove newline from errors

LGTMs: AkihiroSuda cyphar

create.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/urfave/cli"
@@ -55,20 +56,12 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 		if err := checkArgs(context, 1, exactArgs); err != nil {
 			return err
 		}
-		if err := revisePidFile(context); err != nil {
-			return err
+		status, err := startContainer(context, CT_ACT_CREATE, nil)
+		if err == nil {
+			// exit with the container's exit status so any external supervisor
+			// is notified of the exit with the correct exit status.
+			os.Exit(status)
 		}
-		spec, err := setupSpec(context)
-		if err != nil {
-			return err
-		}
-		status, err := startContainer(context, spec, CT_ACT_CREATE, nil)
-		if err != nil {
-			return err
-		}
-		// exit with the container's exit status so any external supervisor is
-		// notified of the exit with the correct exit status.
-		os.Exit(status)
-		return nil
+		return fmt.Errorf("runc create failed: %w", err)
 	},
 }

delete.go

@@ -79,7 +79,7 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
 			if force {
 				return killContainer(container)
 			}
-			return fmt.Errorf("cannot delete container %s that is not stopped: %s\n", id, s)
+			return fmt.Errorf("cannot delete container %s that is not stopped: %s", id, s)
 		}
 
 		return nil

restore.go

@@ -104,15 +104,11 @@ using the runc checkpoint command.`,
 			logrus.Warn("runc checkpoint is untested with rootless containers")
 		}
 
-		spec, err := setupSpec(context)
-		if err != nil {
-			return err
-		}
 		options := criuOptions(context)
 		if err := setEmptyNsMask(context, options); err != nil {
 			return err
 		}
-		status, err := startContainer(context, spec, CT_ACT_RESTORE, options)
+		status, err := startContainer(context, CT_ACT_RESTORE, options)
 		if err != nil {
 			return err
 		}

run.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/urfave/cli"
@@ -68,19 +69,12 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 		if err := checkArgs(context, 1, exactArgs); err != nil {
 			return err
 		}
-		if err := revisePidFile(context); err != nil {
-			return err
-		}
-		spec, err := setupSpec(context)
-		if err != nil {
-			return err
-		}
-		status, err := startContainer(context, spec, CT_ACT_RUN, nil)
+		status, err := startContainer(context, CT_ACT_RUN, nil)
 		if err == nil {
 			// exit with the container's exit status so any external supervisor is
 			// notified of the exit with the correct exit status.
 			os.Exit(status)
 		}
-		return err
+		return fmt.Errorf("runc run failed: %w", err)
 	},
 }

start.go

@@ -48,7 +48,7 @@ your host.`,
 		case libcontainer.Running:
 			return errors.New("cannot start an already running container")
 		default:
-			return fmt.Errorf("cannot start a container in the %s state\n", status)
+			return fmt.Errorf("cannot start a container in the %s state", status)
 		}
 	},
 }

utils_linux.go

@@ -395,7 +395,15 @@ const (
 	CT_ACT_RESTORE
 )
 
-func startContainer(context *cli.Context, spec *specs.Spec, action CtAct, criuOpts *libcontainer.CriuOpts) (int, error) {
+func startContainer(context *cli.Context, action CtAct, criuOpts *libcontainer.CriuOpts) (int, error) {
+	if err := revisePidFile(context); err != nil {
+		return -1, err
+	}
+	spec, err := setupSpec(context)
+	if err != nil {
+		return -1, err
+	}
+
 	id := context.Args().First()
 	if id == "" {
 		return -1, errEmptyID