From 213c1a1a4af39c13e93c2d4fe0c901d831c3ffe7 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 9 Mar 2016 17:48:12 -0800 Subject: [PATCH] Revert "Return proper exit code for exec errors" This reverts commit 6bb653a6e881594eebd9532f6ed95339d3856ac6. Signed-off-by: Michael Crosby --- libcontainer/factory_linux.go | 45 +++++++++---------------- libcontainer/integration/execin_test.go | 38 ++++++++++----------- libcontainer/integration/init_test.go | 20 ----------- start.go | 2 +- utils.go | 26 +++----------- 5 files changed, 39 insertions(+), 92 deletions(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index eb327be7..14e4f33a 100644 --- a/libcontainer/factory_linux.go +++ b/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 -} diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 98dd2ac0..378063e8 100644 --- a/libcontainer/integration/execin_test.go +++ b/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) } } diff --git a/libcontainer/integration/init_test.go b/libcontainer/integration/init_test.go index 7da6096f..eaa6caf6 100644 --- a/libcontainer/integration/init_test.go +++ b/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 ( diff --git a/start.go b/start.go index ec8f217c..127921f6 100644 --- a/start.go +++ b/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. diff --git a/utils.go b/utils.go index c2cf1f2c..9f27965f 100644 --- a/utils.go +++ b/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) }