Merge pull request containerd#26 from mlaventure/forward-wait-err

Refactor Monitor interface to only handle Start() & Wait()

monitor.go

@@ -22,28 +22,13 @@ type Exit struct {
 // These methods should match the methods exposed by exec.Cmd to provide
 // a consistent experience for the caller
 type ProcessMonitor interface {
-	Output(*exec.Cmd) ([]byte, error)
-	CombinedOutput(*exec.Cmd) ([]byte, error)
-	Run(*exec.Cmd) error
 	Start(*exec.Cmd) (chan Exit, error)
 	Wait(*exec.Cmd, chan Exit) (int, error)
 }
 
 type defaultMonitor struct {
 }
 
-func (m *defaultMonitor) Output(c *exec.Cmd) ([]byte, error) {
-	return c.Output()
-}
-
-func (m *defaultMonitor) CombinedOutput(c *exec.Cmd) ([]byte, error) {
-	return c.CombinedOutput()
-}
-
-func (m *defaultMonitor) Run(c *exec.Cmd) error {
-	return c.Run()
-}
-
 func (m *defaultMonitor) Start(c *exec.Cmd) (chan Exit, error) {
 	if err := c.Start(); err != nil {
 		return nil, err

runc.go

@@ -46,7 +46,7 @@ type Runc struct {
 
 // List returns all containers created inside the provided runc root directory
 func (r *Runc) List(context context.Context) ([]*Container, error) {
-	data, err := Monitor.Output(r.command(context, "list", "--format=json"))
+	data, err := cmdOutput(r.command(context, "list", "--format=json"), false)
 	if err != nil {
 		return nil, err
 	}
@@ -59,7 +59,7 @@ func (r *Runc) List(context context.Context) ([]*Container, error) {
 
 // State returns the state for the container provided by id
 func (r *Runc) State(context context.Context, id string) (*Container, error) {
-	data, err := Monitor.CombinedOutput(r.command(context, "state", id))
+	data, err := cmdOutput(r.command(context, "state", id), true)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %s", err, data)
 	}
@@ -128,7 +128,7 @@ func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOp
 	cmd.ExtraFiles = opts.ExtraFiles
 
 	if cmd.Stdout == nil && cmd.Stderr == nil {
-		data, err := Monitor.CombinedOutput(cmd)
+		data, err := cmdOutput(cmd, true)
 		if err != nil {
 			return fmt.Errorf("%s: %s", err, data)
 		}
@@ -145,7 +145,10 @@ func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOp
 			}
 		}
 	}
-	_, err = Monitor.Wait(cmd, ec)
+	status, err := Monitor.Wait(cmd, ec)
+	if err == nil && status != 0 {
+		err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0])
+	}
 	return err
 }
 
@@ -204,7 +207,7 @@ func (r *Runc) Exec(context context.Context, id string, spec specs.Process, opts
 		opts.Set(cmd)
 	}
 	if cmd.Stdout == nil && cmd.Stderr == nil {
-		data, err := Monitor.CombinedOutput(cmd)
+		data, err := cmdOutput(cmd, true)
 		if err != nil {
 			return fmt.Errorf("%s: %s", err, data)
 		}
@@ -363,7 +366,7 @@ func (r *Runc) Resume(context context.Context, id string) error {
 
 // Ps lists all the processes inside the container returning their pids
 func (r *Runc) Ps(context context.Context, id string) ([]int, error) {
-	data, err := Monitor.CombinedOutput(r.command(context, "ps", "--format", "json", id))
+	data, err := cmdOutput(r.command(context, "ps", "--format", "json", id), true)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %s", err, data)
 	}
@@ -546,7 +549,7 @@ type Version struct {
 
 // Version returns the runc and runtime-spec versions
 func (r *Runc) Version(context context.Context) (Version, error) {
-	data, err := Monitor.Output(r.command(context, "--version"))
+	data, err := cmdOutput(r.command(context, "--version"), false)
 	if err != nil {
 		return Version{}, err
 	}
@@ -614,11 +617,39 @@ func (r *Runc) args() (out []string) {
 // <stderr>
 func (r *Runc) runOrError(cmd *exec.Cmd) error {
 	if cmd.Stdout != nil || cmd.Stderr != nil {
-		return Monitor.Run(cmd)
+		ec, err := Monitor.Start(cmd)
+		if err != nil {
+			return err
+		}
+		status, err := Monitor.Wait(cmd, ec)
+		if err == nil && status != 0 {
+			err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0])
+		}
+		return err
 	}
-	data, err := Monitor.CombinedOutput(cmd)
+	data, err := cmdOutput(cmd, true)
 	if err != nil {
 		return fmt.Errorf("%s: %s", err, data)
 	}
 	return nil
 }
+
+func cmdOutput(cmd *exec.Cmd, combined bool) ([]byte, error) {
+	var b bytes.Buffer
+
+	cmd.Stdout = &b
+	if combined {
+		cmd.Stderr = &b
+	}
+	ec, err := Monitor.Start(cmd)
+	if err != nil {
+		return nil, err
+	}
+
+	status, err := Monitor.Wait(cmd, ec)
+	if err == nil && status != 0 {
+		err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0])
+	}
+
+	return b.Bytes(), err
+}