diff --git a/oci/oci.go b/oci/oci.go index 8e0fb8ad..a28009cb 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -155,6 +155,18 @@ type ExecSyncResponse struct { ExitCode int32 } +// ExecSyncError wraps command's streams, exit code and error on ExecSync error. +type ExecSyncError struct { + Stdout bytes.Buffer + Stderr bytes.Buffer + ExitCode int32 + Err error +} + +func (e ExecSyncError) Error() string { + return fmt.Sprintf("command error: %+v, stdout: %s, stderr: %s, exit code %d", e.Err, e.Stdout.Bytes(), e.Stderr.Bytes(), e.ExitCode) +} + // ExecSync execs a command in a container and returns it's stdout, stderr and return code. func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp *ExecSyncResponse, err error) { args := []string{"exec", c.name} @@ -165,11 +177,12 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp cmd.Stderr = &stderrBuf err = cmd.Start() if err != nil { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: -1, - }, err + Err: err, + } } if timeout > 0 { @@ -182,33 +195,37 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp case <-time.After(time.Duration(timeout) * time.Second): err = unix.Kill(cmd.Process.Pid, syscall.SIGKILL) if err != nil && err != syscall.ESRCH { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: -1, - }, fmt.Errorf("failed to kill process on timeout: %v", err) + Err: fmt.Errorf("failed to kill process on timeout: %+v", err), + } } - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: -1, - }, fmt.Errorf("command timed out") + Err: fmt.Errorf("command timed out"), + } case err = <-done: if err != nil { if exitErr, ok := err.(*exec.ExitError); ok { if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: int32(status.ExitStatus()), - }, err + Err: err, + } } } else { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: -1, - }, err + Err: err, + } } } @@ -218,18 +235,20 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp if err != nil { if exitErr, ok := err.(*exec.ExitError); ok { if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: int32(status.ExitStatus()), - }, err + Err: err, + } } } else { - return &ExecSyncResponse{ - Stdout: stdoutBuf.Bytes(), - Stderr: stderrBuf.Bytes(), + return nil, ExecSyncError{ + Stdout: stdoutBuf, + Stderr: stderrBuf, ExitCode: -1, - }, err + Err: err, + } } } diff --git a/server/container_execsync.go b/server/container_execsync.go index ce762737..caab7983 100644 --- a/server/container_execsync.go +++ b/server/container_execsync.go @@ -32,6 +32,9 @@ func (s *Server) ExecSync(ctx context.Context, req *pb.ExecSyncRequest) (*pb.Exe } execResp, err := s.runtime.ExecSync(c, cmd, req.GetTimeout()) + if err != nil { + return nil, err + } resp := &pb.ExecSyncResponse{ Stdout: execResp.Stdout, Stderr: execResp.Stderr, @@ -39,5 +42,5 @@ func (s *Server) ExecSync(ctx context.Context, req *pb.ExecSyncRequest) (*pb.Exe } logrus.Debugf("ExecSyncResponse: %+v", resp) - return resp, err + return resp, nil } diff --git a/test/ctr.bats b/test/ctr.bats index 4ace03f3..e4857853 100644 --- a/test/ctr.bats +++ b/test/ctr.bats @@ -360,3 +360,31 @@ function teardown() { cleanup_pods stop_ocid } + +@test "ctr execsync failure" { + # this test requires docker, thus it can't yet be run in a container + if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here + skip "cannot yet run this test in a container, use sudo make localintegration" + fi + + start_ocid + run ocic pod create --config "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run ocic ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run ocic ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run ocic ctr execsync --id "$ctr_id" doesnotexist + echo "$output" + [ "$status" -ne 0 ] + [[ "$output" =~ "executable file not found in" ]] + + cleanup_ctrs + cleanup_pods + stop_ocid +}