Merge pull request #591 from crosbymichael/exec-errors

Return proper exit code for exec errors

libcontainer/factory_linux.go

@@ -227,32 +227,40 @@ func (l *LinuxFactory) StartInitialization() (err error) {
 		pipe = os.NewFile(uintptr(pipefd), "pipe")
 		it   = initType(os.Getenv("_LIBCONTAINER_INITTYPE"))
 	)
+	defer pipe.Close()
 	// clear the current process's environment to clean any libcontainer
 	// specific env vars.
 	os.Clearenv()
-	var i initer
+	i, err := newContainerInit(it, pipe)
-	defer func() {
+	if err != nil {
-		// We have an error during the initialization of the container's init,
+		l.sendError(nil, pipe, err)
-		// send it back to the parent process in the form of an initError.
+		return err
-		// If container's init successed, syscall.Exec will not return, hence
+	}
-		// this defer function will never be called.
+	if err := i.Init(); err != nil {
+		if !isExecError(err) {
+			l.sendError(i, pipe, err)
+		}
+		return err
+	}
+	return nil
+}
+
+func (l *LinuxFactory) sendError(i initer, pipe *os.File, err error) {
+	// We have an error during the initialization of the container's init,
+	// send it back to the parent process in the form of an initError.
+	// If container's init successed, syscall.Exec will not return, hence
+	// this defer function will never be called.
+	if i != nil {
 		if _, ok := i.(*linuxStandardInit); ok {
 			//  Synchronisation only necessary for standard init.
 			if err := utils.WriteJSON(pipe, syncT{procError}); err != nil {
 				panic(err)
 			}
 		}
-		if err := utils.WriteJSON(pipe, newSystemError(err)); err != nil {
-			panic(err)
-		}
-		// ensure that this pipe is always closed
-		pipe.Close()
-	}()
-	i, err = newContainerInit(it, pipe)
-	if err != nil {
-		return err
 	}
-	return i.Init()
+	if err := utils.WriteJSON(pipe, newSystemError(err)); err != nil {
+		panic(err)
+	}
 }
 
 func (l *LinuxFactory) loadState(root string) (*State, error) {
@@ -280,3 +288,8 @@ func (l *LinuxFactory) validateID(id string) error {
 	}
 	return nil
 }
+
+func isExecError(err error) bool {
+	_, ok := err.(*exec.Error)
+	return ok
+}

libcontainer/integration/execin_test.go

@@ -4,8 +4,10 @@ import (
 	"bytes"
 	"io"
 	"os"
+	"os/exec"
 	"strconv"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -138,25 +140,27 @@ func TestExecInError(t *testing.T) {
 	}()
 	ok(t, err)
 
-	for i := 0; i < 42; i++ {
+	var out bytes.Buffer
-		var out bytes.Buffer
+	unexistent := &libcontainer.Process{
-		unexistent := &libcontainer.Process{
+		Cwd:    "/",
-			Cwd:    "/",
+		Args:   []string{"unexistent"},
-			Args:   []string{"unexistent"},
+		Env:    standardEnvironment,
-			Env:    standardEnvironment,
+		Stdout: &out,
-			Stdout: &out,
+		Stderr: &out,
-		}
+	}
-		err = container.Start(unexistent)
+	err = container.Start(unexistent)
-		if err == nil {
+	if err != nil {
-			t.Fatal("Should be an error")
+		t.Fatal(err)
-		}
+	}
-		if !strings.Contains(err.Error(), "executable file not found") {
+	ws, err := unexistent.Wait()
-			t.Fatalf("Should be error about not found executable, got %s", err)
+	if err != nil {
-		}
+		if _, ok := err.(*exec.ExitError); !ok {
-		if !bytes.Contains(out.Bytes(), []byte("executable file not found")) {
+			t.Fatal(err)
-			t.Fatalf("executable file not found error not delivered to stdio:\n%s", out.String())
 		}
 	}
+	if s := ws.Sys().(syscall.WaitStatus).ExitStatus(); s != 127 {
+		t.Fatalf("expected wait status of 127 but received %d", s)
+	}
 }
 
 func TestExecInTTY(t *testing.T) {

libcontainer/integration/init_test.go

@@ -1,8 +1,11 @@
 package integration
 
 import (
+	"fmt"
 	"os"
+	"os/exec"
 	"runtime"
+	"strconv"
 	"testing"
 
 	"github.com/Sirupsen/logrus"
@@ -24,8 +27,25 @@ func init() {
 		logrus.Fatalf("unable to initialize for container: %s", err)
 	}
 	if err := factory.StartInitialization(); err != nil {
+		// return proper unix error codes
+		if exerr, ok := err.(*exec.Error); ok {
+			switch exerr.Err {
+			case os.ErrPermission:
+				fmt.Fprintln(os.Stderr, err)
+				os.Exit(126)
+			case exec.ErrNotFound:
+				fmt.Fprintln(os.Stderr, err)
+				os.Exit(127)
+			default:
+				if os.IsNotExist(exerr.Err) {
+					fmt.Fprintf(os.Stderr, "exec: %s: %v\n", strconv.Quote(exerr.Name), os.ErrNotExist)
+					os.Exit(127)
+				}
+			}
+		}
 		logrus.Fatal(err)
 	}
+	panic("init: init failed to start contianer")
 }
 
 var (

start.go

@@ -84,7 +84,7 @@ func init() {
 		if err := factory.StartInitialization(); err != nil {
 			fatal(err)
 		}
-		panic("--this line should have never been executed, congratulations--")
+		panic("libcontainer: container init failed to exec")
 	}
 }
 

utils.go

@@ -6,7 +6,9 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"strconv"
 	"syscall"
 
 	"github.com/Sirupsen/logrus"
@@ -161,6 +163,22 @@ 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) {
+	// return proper unix error codes
+	if exerr, ok := err.(*exec.Error); ok {
+		switch exerr.Err {
+		case os.ErrPermission:
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(126)
+		case exec.ErrNotFound:
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(127)
+		default:
+			if os.IsNotExist(exerr.Err) {
+				fmt.Fprintf(os.Stderr, "exec: %s: %v\n", strconv.Quote(exerr.Name), os.ErrNotExist)
+				os.Exit(127)
+			}
+		}
+	}
 	if lerr, ok := err.(libcontainer.Error); ok {
 		lerr.Detail(os.Stderr)
 		os.Exit(1)