Revert "Return proper exit code for exec errors"

This reverts commit 6bb653a6e8.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

libcontainer/factory_linux.go

@@ -227,40 +227,32 @@ 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()
-	i, err := newContainerInit(it, pipe)
-	if err != nil {
-		l.sendError(nil, pipe, err)
-		return err
-	}
-	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 {
+	var i initer
+	defer func() {
+		// 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 _, 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
 	}
-	if err := utils.WriteJSON(pipe, newSystemError(err)); err != nil {
-		panic(err)
-	}
+	return i.Init()
 }
 
 func (l *LinuxFactory) loadState(root string) (*State, error) {
@@ -288,8 +280,3 @@ 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

@@ -5,10 +5,8 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"strconv"
 	"strings"
-	"syscall"
 	"testing"
 	"time"
 
@@ -142,26 +140,24 @@ func TestExecInError(t *testing.T) {
 	}()
 	ok(t, err)
 
-	var out bytes.Buffer
-	unexistent := &libcontainer.Process{
-		Cwd:    "/",
-		Args:   []string{"unexistent"},
-		Env:    standardEnvironment,
-		Stdout: &out,
-		Stderr: &out,
-	}
-	err = container.Start(unexistent)
-	if err != nil {
-		t.Fatal(err)
-	}
-	ws, err := unexistent.Wait()
-	if err != nil {
-		if _, ok := err.(*exec.ExitError); !ok {
-			t.Fatal(err)
+	for i := 0; i < 42; i++ {
+		var out bytes.Buffer
+		unexistent := &libcontainer.Process{
+			Cwd:    "/",
+			Args:   []string{"unexistent"},
+			Env:    standardEnvironment,
+			Stdout: &out,
+		}
+		err = container.Start(unexistent)
+		if err == nil {
+			t.Fatal("Should be an error")
+		}
+		if !strings.Contains(err.Error(), "executable file not found") {
+			t.Fatalf("Should be error about not found executable, got %s", err)
+		}
+		if !bytes.Contains(out.Bytes(), []byte("executable file not found")) {
+			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)
 	}
 }
 

libcontainer/integration/init_test.go

@@ -1,11 +1,8 @@
 package integration
 
 import (
-	"fmt"
 	"os"
-	"os/exec"
 	"runtime"
-	"strconv"
 	"testing"
 
 	"github.com/Sirupsen/logrus"
@@ -27,25 +24,8 @@ 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

@@ -67,7 +67,7 @@ is a directory with a specification file and a root filesystem.`,
 
 		status, err := startContainer(context, spec)
 		if err != nil {
-			fatalf("Container start failed: %v", err)
+			fatal(err)
 		}
 		// exit with the container's exit status so any external supervisor is
 		// notified of the exit with the correct exit status.

utils.go

@@ -6,9 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"strconv"
 	"syscall"
 
 	"github.com/Sirupsen/logrus"
@@ -163,9 +161,11 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
 	if err != nil {
 		return nil, err
 	}
-	logAbs, err := filepath.Abs(context.GlobalString("log"))
-	if err != nil {
-		return nil, err
+	var logAbs string
+	if l := context.GlobalString("log"); l != "" {
+		if logAbs, err = filepath.Abs(context.GlobalString("log")); err != nil {
+			return nil, err
+		}
 	}
 	return libcontainer.New(abs, libcontainer.Cgroupfs, func(l *libcontainer.LinuxFactory) error {
 		l.CriuPath = context.GlobalString("criu")
@@ -198,22 +198,6 @@ func getContainer(context *cli.Context) (libcontainer.Container, error) {
 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 {
-		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)
-			}
-		}
-	}
 	fmt.Fprintln(os.Stderr, err)
 	os.Exit(1)
 }