From edcc94f066bb95fcd9bb603dd9d1ccb3cd696690 Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Fri, 27 Feb 2015 03:47:57 -0500 Subject: [PATCH 1/4] do not pass nil to genericError currently genericError constructors require not-nil error to be able to read its Error() message Signed-off-by: Daniel, Dao Quang Minh --- container_linux.go | 2 +- process.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/container_linux.go b/container_linux.go index 3ea08db26..8fffd39d1 100644 --- a/container_linux.go +++ b/container_linux.go @@ -206,7 +206,7 @@ func (c *linuxContainer) Destroy() error { return err } if status != Destroyed { - return newGenericError(nil, ContainerNotStopped) + return newGenericError(fmt.Errorf("container is not destroyed"), ContainerNotStopped) } if !c.config.Namespaces.Contains(configs.NEWPID) { if err := killCgroupProcesses(c.cgroupManager); err != nil { diff --git a/process.go b/process.go index 8ae741639..1f769abb4 100644 --- a/process.go +++ b/process.go @@ -1,6 +1,7 @@ package libcontainer import ( + "fmt" "io" "os" ) @@ -46,7 +47,7 @@ type Process struct { // Wait releases any resources associated with the Process func (p Process) Wait() (*os.ProcessState, error) { if p.ops == nil { - return nil, newGenericError(nil, ProcessNotExecuted) + return nil, newGenericError(fmt.Errorf("invalid process"), ProcessNotExecuted) } return p.ops.wait() } @@ -54,7 +55,7 @@ func (p Process) Wait() (*os.ProcessState, error) { // Pid returns the process ID func (p Process) Pid() (int, error) { if p.ops == nil { - return -1, newGenericError(nil, ProcessNotExecuted) + return -1, newGenericError(fmt.Errorf("invalid process"), ProcessNotExecuted) } return p.ops.pid(), nil } @@ -62,7 +63,7 @@ func (p Process) Pid() (int, error) { // Signal sends a signal to the Process. func (p Process) Signal(sig os.Signal) error { if p.ops == nil { - return newGenericError(nil, ProcessNotExecuted) + return newGenericError(fmt.Errorf("invalid process"), ProcessNotExecuted) } return p.ops.signal(sig) } From 526a39ebdaafa12518a9c12bb1783661860a46da Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Fri, 27 Feb 2015 03:50:20 -0500 Subject: [PATCH 2/4] genericError constructors can accept nil error if the error is nil, we do not populate generic error's message, but the constructor will still return a valid error Signed-off-by: Daniel, Dao Quang Minh --- generic_error.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/generic_error.go b/generic_error.go index 05ab0a9d3..ff4d7248d 100644 --- a/generic_error.go +++ b/generic_error.go @@ -25,26 +25,32 @@ func newGenericError(err error, c ErrorCode) Error { if le, ok := err.(Error); ok { return le } - return &genericError{ + gerr := &genericError{ Timestamp: time.Now(), Err: err, - Message: err.Error(), ECode: c, Stack: stacktrace.Capture(1), } + if err != nil { + gerr.Message = err.Error() + } + return gerr } func newSystemError(err error) Error { if le, ok := err.(Error); ok { return le } - return &genericError{ + gerr := &genericError{ Timestamp: time.Now(), Err: err, ECode: SystemError, - Message: err.Error(), Stack: stacktrace.Capture(1), } + if err != nil { + gerr.Message = err.Error() + } + return gerr } type genericError struct { From 71fc3faf6b074a0fdca1fa6a61c936178aec9235 Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Fri, 27 Feb 2015 03:56:08 -0500 Subject: [PATCH 3/4] setup setns process environment with os.Environ() instead of taking the raw config.Env, we use `os.Environ()` which works correctly because the environment has been setup properly with `newContainerInit` Signed-off-by: Daniel, Dao Quang Minh --- integration/execin_test.go | 68 ++++++++++++++++++++++++++++++++++++++ setns_init_linux.go | 4 ++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/integration/execin_test.go b/integration/execin_test.go index 3a1c53875..ace906faa 100644 --- a/integration/execin_test.go +++ b/integration/execin_test.go @@ -66,6 +66,74 @@ func TestExecIn(t *testing.T) { } } +func TestExecInEnvironment(t *testing.T) { + if testing.Short() { + return + } + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + config := newTemplateConfig(rootfs) + container, err := newContainer(config) + if err != nil { + t.Fatal(err) + } + defer container.Destroy() + + // Execute a first process in the container + stdinR, stdinW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + process := &libcontainer.Process{ + Args: []string{"cat"}, + Env: standardEnvironment, + Stdin: stdinR, + } + err = container.Start(process) + stdinR.Close() + defer stdinW.Close() + if err != nil { + t.Fatal(err) + } + + buffers := newStdBuffers() + ps := &libcontainer.Process{ + Args: []string{"env"}, + Env: []string{ + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "DEBUG=true", + "DEBUG=false", + "ENV=test", + }, + Stdin: buffers.Stdin, + Stdout: buffers.Stdout, + Stderr: buffers.Stderr, + } + err = container.Start(ps) + if err != nil { + t.Fatal(err) + } + if _, err := ps.Wait(); err != nil { + out := buffers.Stdout.String() + t.Fatal(err, out) + } + stdinW.Close() + if _, err := process.Wait(); err != nil { + t.Log(err) + } + out := buffers.Stdout.String() + // check execin's process environment + if !strings.Contains(out, "DEBUG=false") || + !strings.Contains(out, "ENV=test") || + !strings.Contains(out, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") || + strings.Contains(out, "DEBUG=true") { + t.Fatalf("unexpected running process, output %q", out) + } +} + func TestExecInRlimit(t *testing.T) { if testing.Short() { return diff --git a/setns_init_linux.go b/setns_init_linux.go index 2d91475bf..f77219d27 100644 --- a/setns_init_linux.go +++ b/setns_init_linux.go @@ -3,6 +3,8 @@ package libcontainer import ( + "os" + "github.com/docker/libcontainer/apparmor" "github.com/docker/libcontainer/label" "github.com/docker/libcontainer/system" @@ -29,5 +31,5 @@ func (l *linuxSetnsInit) Init() error { return err } } - return system.Execv(l.config.Args[0], l.config.Args[0:], l.config.Env) + return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ()) } From 4c0ad2114d8295ff3b6118605c62e0c4fb0d942a Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Fri, 27 Feb 2015 04:07:57 -0500 Subject: [PATCH 4/4] setup init process environment with os.Environ() instead of taking the raw config.Env, we use `os.Environ()` which works correctly because the environment has been setup properly with `newContainerInit` Signed-off-by: Daniel, Dao Quang Minh --- integration/exec_test.go | 43 ++++++++++++++++++++++++++++++++++++++++ standard_init_linux.go | 3 ++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/integration/exec_test.go b/integration/exec_test.go index cc3740d62..f436133ec 100644 --- a/integration/exec_test.go +++ b/integration/exec_test.go @@ -56,6 +56,49 @@ func testExecPS(t *testing.T, userns bool) { } } +func TestExecEnvironment(t *testing.T) { + if testing.Short() { + return + } + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + + config := newTemplateConfig(rootfs) + container, err := newContainer(config) + if err != nil { + t.Fatal(err) + } + defer container.Destroy() + + buffers := newStdBuffers() + process := &libcontainer.Process{ + Args: []string{"env"}, + Env: []string{ + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "TERM=dummy", + "TERM=xterm", + }, + Stdin: buffers.Stdin, + Stdout: buffers.Stdout, + Stderr: buffers.Stderr, + } + + if err := container.Start(process); err != nil { + t.Fatal(err) + } + waitProcess(process, t) + + out := buffers.Stdout.String() + if !strings.Contains(out, "TERM=xterm") || + !strings.Contains(out, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") || + strings.Contains(out, "TERM=dummy") { + t.Fatalf("unexpected environment, output %q", out) + } +} + func TestIPCPrivate(t *testing.T) { if testing.Short() { return diff --git a/standard_init_linux.go b/standard_init_linux.go index db721b8d7..29619d3cd 100644 --- a/standard_init_linux.go +++ b/standard_init_linux.go @@ -3,6 +3,7 @@ package libcontainer import ( + "os" "syscall" "github.com/docker/libcontainer/apparmor" @@ -89,5 +90,5 @@ func (l *linuxStandardInit) Init() error { if syscall.Getppid() == 1 { return syscall.Kill(syscall.Getpid(), syscall.SIGKILL) } - return system.Execv(l.config.Args[0], l.config.Args[0:], l.config.Env) + return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ()) }