Merge pull request #212 from runcom/execsync-fix

execsync: return proper error description

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{
+		return nil, ExecSyncError{
-			Stdout:   stdoutBuf.Bytes(),
+			Stdout:   stdoutBuf,
-			Stderr:   stderrBuf.Bytes(),
+			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{
+				return nil, ExecSyncError{
-					Stdout:   stdoutBuf.Bytes(),
+					Stdout:   stdoutBuf,
-					Stderr:   stderrBuf.Bytes(),
+					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(),
+			return nil, ExecSyncError{
-				Stderr:   stderrBuf.Bytes(),
+				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{
+						return nil, ExecSyncError{
-							Stdout:   stdoutBuf.Bytes(),
+							Stdout:   stdoutBuf,
-							Stderr:   stderrBuf.Bytes(),
+							Stderr:   stderrBuf,
 							ExitCode: int32(status.ExitStatus()),
-						}, err
+							Err:      err,
+						}
 					}
 				} else {
-					return &ExecSyncResponse{
+					return nil, ExecSyncError{
-						Stdout:   stdoutBuf.Bytes(),
+						Stdout:   stdoutBuf,
-						Stderr:   stderrBuf.Bytes(),
+						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{
+					return nil, ExecSyncError{
-						Stdout:   stdoutBuf.Bytes(),
+						Stdout:   stdoutBuf,
-						Stderr:   stderrBuf.Bytes(),
+						Stderr:   stderrBuf,
 						ExitCode: int32(status.ExitStatus()),
-					}, err
+						Err:      err,
+					}
 				}
 			} else {
-				return &ExecSyncResponse{
+				return nil, ExecSyncError{
-					Stdout:   stdoutBuf.Bytes(),
+					Stdout:   stdoutBuf,
-					Stderr:   stderrBuf.Bytes(),
+					Stderr:   stderrBuf,
 					ExitCode: -1,
-				}, err
+					Err:      err,
+				}
 			}
 
 		}

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
 }

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
+}