Merge pull request #628 from crosbymichael/log-errors

Improve error handling in runc

events.go

@@ -35,7 +35,7 @@ information is displayed once every 5 seconds.`,
 	Action: func(context *cli.Context) {
 		container, err := getContainer(context)
 		if err != nil {
-			logrus.Fatal(err)
+			fatal(err)
 		}
 		var (
 			stats  = make(chan *libcontainer.Stats, 1)
@@ -74,7 +74,7 @@ information is displayed once every 5 seconds.`,
 		}()
 		n, err := container.NotifyOOM()
 		if err != nil {
-			logrus.Fatal(err)
+			fatal(err)
 		}
 		for {
 			select {

exec.go

@@ -9,7 +9,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/codegangsta/cli"
 	"github.com/opencontainers/specs"
 )
@@ -80,11 +79,11 @@ following will output a list of processes running in the container:
 	},
 	Action: func(context *cli.Context) {
 		if os.Geteuid() != 0 {
-			logrus.Fatal("runc should be run as root")
+			fatalf("runc should be run as root")
 		}
 		status, err := execProcess(context)
 		if err != nil {
-			logrus.Fatalf("exec failed: %v", err)
+			fatalf("exec failed: %v", err)
 		}
 		os.Exit(status)
 	},

main.go

@@ -49,6 +49,7 @@ func main() {
 		},
 		cli.StringFlag{
 			Name:  "log",
+			Value: "/dev/null",
 			Usage: "set the log file path where internal debug information is written",
 		},
 		cli.StringFlag{
@@ -72,6 +73,7 @@ func main() {
 		deleteCommand,
 		eventsCommand,
 		execCommand,
+		initCommand,
 		killCommand,
 		listCommand,
 		pauseCommand,
@@ -86,7 +88,7 @@ func main() {
 			logrus.SetLevel(logrus.DebugLevel)
 		}
 		if path := context.GlobalString("log"); path != "" {
-			f, err := os.Create(path)
+			f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 			if err != nil {
 				return err
 			}
@@ -103,6 +105,6 @@ func main() {
 		return nil
 	}
 	if err := app.Run(os.Args); err != nil {
-		logrus.Fatal(err)
+		fatal(err)
 	}
 }

main_unsupported.go

@@ -2,10 +2,7 @@
 
 package main
 
-import (
-	"github.com/Sirupsen/logrus"
-	"github.com/codegangsta/cli"
-)
+import "github.com/codegangsta/cli"
 
 var (
 	checkpointCommand cli.Command
@@ -16,5 +13,5 @@ var (
 )
 
 func runAction(*cli.Context) {
-	logrus.Fatal("Current OS is not supported yet")
+	fatalf("Current OS is not supported yet")
 }

restore.go

@@ -3,7 +3,6 @@
 package main
 
 import (
-	"fmt"
 	"os"
 	"syscall"
 
@@ -124,7 +123,7 @@ func restoreContainer(context *cli.Context, spec *specs.LinuxSpec, config *confi
 		logrus.Error(err)
 	}
 	if status == libcontainer.Running {
-		fatal(fmt.Errorf("Container with id %s already running", id))
+		fatalf("Container with id %s already running", id)
 	}
 
 	setManageCgroupsMode(context, options)

spec.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 	"syscall"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/codegangsta/cli"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
@@ -163,14 +162,14 @@ var specCommand = cli.Command{
 			}
 		}
 		if err := checkNoFile(specConfig); err != nil {
-			logrus.Fatal(err)
+			fatal(err)
 		}
 		data, err := json.MarshalIndent(&spec, "", "\t")
 		if err != nil {
-			logrus.Fatal(err)
+			fatal(err)
 		}
 		if err := ioutil.WriteFile(specConfig, data, 0666); err != nil {
-			logrus.Fatal(err)
+			fatal(err)
 		}
 	},
 }

start.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"runtime"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/codegangsta/cli"
 	"github.com/coreos/go-systemd/activation"
 	"github.com/opencontainers/runc/libcontainer"
@@ -63,12 +62,12 @@ is a directory with a specification file and a root filesystem.`,
 		}
 
 		if os.Geteuid() != 0 {
-			logrus.Fatal("runc should be run as root")
+			fatalf("runc should be run as root")
 		}
 
 		status, err := startContainer(context, spec)
 		if err != nil {
-			logrus.Fatalf("Container start failed: %v", err)
+			fatalf("Container start failed: %v", err)
 		}
 		// exit with the container's exit status so any external supervisor is
 		// notified of the exit with the correct exit status.
@@ -76,8 +75,12 @@ is a directory with a specification file and a root filesystem.`,
 	},
 }
 
-func init() {
-	if len(os.Args) > 1 && os.Args[1] == "init" {
+var initCommand = cli.Command{
+	Name: "init",
+	Usage: `init is used to initialize the containers namespaces and launch the users process.
+    This command should not be called outside of runc.
+    `,
+	Action: func(context *cli.Context) {
 		runtime.GOMAXPROCS(1)
 		runtime.LockOSThread()
 		factory, _ := libcontainer.New("")
@@ -85,7 +88,7 @@ func init() {
 			fatal(err)
 		}
 		panic("libcontainer: container init failed to exec")
-	}
+	},
 }
 
 func startContainer(context *cli.Context, spec *specs.LinuxSpec) (int, error) {

utils.go

@@ -152,15 +152,31 @@ func containerPreload(context *cli.Context) error {
 
 // loadFactory returns the configured factory instance for execing containers.
 func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
-	root := context.GlobalString("root")
+	var (
+		debug = "false"
+		root  = context.GlobalString("root")
+	)
+	if context.GlobalBool("debug") {
+		debug = "true"
+	}
 	abs, err := filepath.Abs(root)
 	if err != nil {
 		return nil, err
 	}
+	logAbs, err := filepath.Abs(context.GlobalString("log"))
+	if err != nil {
+		return nil, err
+	}
 	return libcontainer.New(abs, libcontainer.Cgroupfs, func(l *libcontainer.LinuxFactory) error {
 		l.CriuPath = context.GlobalString("criu")
 		return nil
-	})
+	},
+		libcontainer.InitArgs(os.Args[0],
+			"--log", logAbs,
+			"--log-format", context.GlobalString("log-format"),
+			fmt.Sprintf("--debug=%s", debug),
+			"init"),
+	)
 }
 
 // getContainer returns the specified container instance by loading it from state
@@ -180,6 +196,8 @@ func getContainer(context *cli.Context) (libcontainer.Container, error) {
 // fatal prints the error's details if it is a libcontainer specific error type
 // then exits the program with an exit status of 1.
 func fatal(err error) {
+	// make sure the error is written to the logger
+	logrus.Error(err)
 	// return proper unix error codes
 	if exerr, ok := err.(*exec.Error); ok {
 		switch exerr.Err {
@@ -196,14 +214,14 @@ func fatal(err error) {
 			}
 		}
 	}
-	if lerr, ok := err.(libcontainer.Error); ok {
-		lerr.Detail(os.Stderr)
-		os.Exit(1)
-	}
 	fmt.Fprintln(os.Stderr, err)
 	os.Exit(1)
 }
 
+func fatalf(t string, v ...interface{}) {
+	fatal(fmt.Errorf(t, v...))
+}
+
 func getDefaultImagePath(context *cli.Context) string {
 	cwd, err := os.Getwd()
 	if err != nil {