Merge pull request #1731 from coryb/issue-1714

add tty support for runc executor

client/build_test.go

@@ -3,10 +3,12 @@ package client
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -37,6 +39,8 @@ func TestClientGatewayIntegration(t *testing.T) {
 		testClientGatewayContainerPID1Fail,
 		testClientGatewayContainerPID1Exit,
 		testClientGatewayContainerMounts,
+		testClientGatewayContainerPID1Tty,
+		testClientGatewayContainerExecTty,
 	}, integration.WithMirroredImages(integration.OfficialImages("busybox:latest")))
 }
 
@@ -718,6 +722,234 @@ func testClientGatewayContainerMounts(t *testing.T, sb integration.Sandbox) {
 	checkAllReleasable(t, c, sb, true)
 }
 
+// testClientGatewayContainerPID1Tty is testing that we can get a tty via
+// a container pid1, executor.Run
+func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) {
+	requiresLinux(t)
+	ctx := context.TODO()
+
+	c, err := New(ctx, sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	product := "buildkit_test"
+
+	inputR, inputW := io.Pipe()
+	output := bytes.NewBuffer(nil)
+
+	b := func(ctx context.Context, c client.Client) (*client.Result, error) {
+		ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
+		defer timeout()
+
+		st := llb.Image("busybox:latest")
+
+		def, err := st.Marshal(ctx)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal state")
+		}
+
+		r, err := c.Solve(ctx, client.SolveRequest{
+			Definition: def.ToPB(),
+		})
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to solve")
+		}
+
+		ctr, err := c.NewContainer(ctx, client.NewContainerRequest{
+			Mounts: []client.Mount{{
+				Dest:      "/",
+				MountType: pb.MountType_BIND,
+				Ref:       r.Ref,
+			}},
+		})
+		require.NoError(t, err)
+		defer ctr.Release(ctx)
+
+		prompt := newTestPrompt(ctx, t, inputW, output)
+		pid1, err := ctr.Start(ctx, client.StartRequest{
+			Args:   []string{"sh"},
+			Tty:    true,
+			Stdin:  inputR,
+			Stdout: &nopCloser{output},
+			Stderr: &nopCloser{output},
+			Env:    []string{fmt.Sprintf("PS1=%s", prompt.String())},
+		})
+		require.NoError(t, err)
+		err = pid1.Resize(ctx, client.WinSize{Rows: 40, Cols: 80})
+		require.NoError(t, err)
+		prompt.SendExpect("ttysize", "80 40")
+		prompt.Send("cd /tmp")
+		prompt.SendExpect("pwd", "/tmp")
+		prompt.Send("echo foobar > newfile")
+		prompt.SendExpect("cat /tmp/newfile", "foobar")
+		err = pid1.Resize(ctx, client.WinSize{Rows: 60, Cols: 100})
+		require.NoError(t, err)
+		prompt.SendExpect("ttysize", "100 60")
+		prompt.SendExit(99)
+
+		err = pid1.Wait()
+		var exitError *errdefs.ExitError
+		require.True(t, errors.As(err, &exitError))
+		require.Equal(t, uint32(99), exitError.ExitCode)
+
+		return &client.Result{}, err
+	}
+
+	_, err = c.Build(ctx, SolveOpt{}, product, b, nil)
+	require.Error(t, err)
+
+	inputW.Close()
+	inputR.Close()
+
+	checkAllReleasable(t, c, sb, true)
+}
+
+type testPrompt struct {
+	ctx    context.Context
+	t      *testing.T
+	output *bytes.Buffer
+	input  io.Writer
+	prompt string
+	pos    int
+}
+
+func newTestPrompt(ctx context.Context, t *testing.T, input io.Writer, output *bytes.Buffer) *testPrompt {
+	return &testPrompt{
+		ctx:    ctx,
+		t:      t,
+		input:  input,
+		output: output,
+		prompt: "% ",
+	}
+}
+
+func (p *testPrompt) String() string { return p.prompt }
+
+func (p *testPrompt) SendExit(status int) {
+	p.input.Write([]byte(fmt.Sprintf("exit %d\n", status)))
+}
+
+func (p *testPrompt) Send(cmd string) {
+	p.input.Write([]byte(cmd + "\n"))
+	p.wait(p.prompt)
+}
+
+func (p *testPrompt) SendExpect(cmd, expected string) {
+	for {
+		p.input.Write([]byte(cmd + "\n"))
+		response := p.wait(p.prompt)
+		if strings.Contains(response, expected) {
+			return
+		}
+	}
+}
+
+func (p *testPrompt) wait(msg string) string {
+	for {
+		newOutput := p.output.String()[p.pos:]
+		if strings.Contains(newOutput, msg) {
+			p.pos += len(newOutput)
+			return newOutput
+		}
+		select {
+		case <-p.ctx.Done():
+			p.t.Logf("Output at timeout: %s", p.output.String())
+			p.t.Fatalf("Timeout waiting for %q", msg)
+		case <-time.After(100 * time.Millisecond):
+		}
+	}
+}
+
+// testClientGatewayContainerExecTty is testing that we can get a tty via
+// executor.Exec (secondary process)
+func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) {
+	if sb.Rootless() {
+		// TODO fix this
+		// We get `panic: cannot statfs cgroup root` when running this test
+		// with runc-rootless
+		t.Skip("Skipping runc-rootless for cgroup error")
+	}
+	requiresLinux(t)
+	ctx := context.TODO()
+
+	c, err := New(ctx, sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	product := "buildkit_test"
+
+	inputR, inputW := io.Pipe()
+	output := bytes.NewBuffer(nil)
+	b := func(ctx context.Context, c client.Client) (*client.Result, error) {
+		ctx, timeout := context.WithTimeout(ctx, 10*time.Second)
+		defer timeout()
+		st := llb.Image("busybox:latest")
+
+		def, err := st.Marshal(ctx)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal state")
+		}
+
+		r, err := c.Solve(ctx, client.SolveRequest{
+			Definition: def.ToPB(),
+		})
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to solve")
+		}
+
+		ctr, err := c.NewContainer(ctx, client.NewContainerRequest{
+			Mounts: []client.Mount{{
+				Dest:      "/",
+				MountType: pb.MountType_BIND,
+				Ref:       r.Ref,
+			}},
+		})
+		require.NoError(t, err)
+
+		pid1, err := ctr.Start(ctx, client.StartRequest{
+			Args: []string{"sleep", "10"},
+		})
+		require.NoError(t, err)
+
+		defer pid1.Wait()
+		defer ctr.Release(ctx)
+
+		prompt := newTestPrompt(ctx, t, inputW, output)
+		pid2, err := ctr.Start(ctx, client.StartRequest{
+			Args:   []string{"sh"},
+			Tty:    true,
+			Stdin:  inputR,
+			Stdout: &nopCloser{output},
+			Stderr: &nopCloser{output},
+			Env:    []string{fmt.Sprintf("PS1=%s", prompt.String())},
+		})
+		require.NoError(t, err)
+
+		err = pid2.Resize(ctx, client.WinSize{Rows: 40, Cols: 80})
+		require.NoError(t, err)
+		prompt.SendExpect("ttysize", "80 40")
+		prompt.Send("cd /tmp")
+		prompt.SendExpect("pwd", "/tmp")
+		prompt.Send("echo foobar > newfile")
+		prompt.SendExpect("cat /tmp/newfile", "foobar")
+		err = pid2.Resize(ctx, client.WinSize{Rows: 60, Cols: 100})
+		require.NoError(t, err)
+		prompt.SendExpect("ttysize", "100 60")
+		prompt.SendExit(99)
+
+		return &client.Result{}, pid2.Wait()
+	}
+
+	_, err = c.Build(ctx, SolveOpt{}, product, b, nil)
+	require.Error(t, err)
+	require.Regexp(t, "exit code: 99|runc did not terminate successfully", err.Error())
+
+	inputW.Close()
+	inputR.Close()
+
+	checkAllReleasable(t, c, sb, true)
+}
+
 type nopCloser struct {
 	io.Writer
 }

executor/runcexecutor/executor.go

@@ -272,10 +272,7 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root cache.Mountable,
 		}
 	}
 
-	if meta.Tty {
-		return errors.New("tty with runc not implemented")
-	}
-
+	spec.Process.Terminal = meta.Tty
 	spec.Process.OOMScoreAdj = w.oomScoreAdj
 	if w.rootless {
 		if err := rootlessspecconv.ToRootless(spec); err != nil {
@@ -326,10 +323,8 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root cache.Mountable,
 			close(started)
 		})
 	}
-	status, err := w.runc.Run(runCtx, id, bundle, &runc.CreateOpts{
-		IO:      &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr},
-		NoPivot: w.noPivot,
-	})
+
+	status, err := w.run(runCtx, id, bundle, process)
 	close(ended)
 
 	if status != 0 || err != nil {
@@ -413,21 +408,14 @@ func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.Pro
 		spec.Process.Env = process.Meta.Env
 	}
 
-	err = w.runc.Exec(ctx, id, *spec.Process, &runc.ExecOpts{
-		IO: &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr},
-	})
-
-	var exitError *exec.ExitError
-	if errors.As(err, &exitError) {
-		err = &errdefs.ExitError{
-			ExitCode: uint32(exitError.ExitCode()),
-			Err:      err,
-		}
-		return err
-	} else if err != nil {
-		return err
+	status, err := w.exec(ctx, id, state.Bundle, spec.Process, process)
+	if status == 0 && err == nil {
+		return nil
+	}
+	return &errdefs.ExitError{
+		ExitCode: uint32(status),
+		Err:      err,
 	}
-	return nil
 }
 
 type forwardIO struct {

executor/runcexecutor/executor_common.go

@@ -0,0 +1,44 @@
+// +build !linux
+
+package runcexecutor
+
+import (
+	"context"
+	"os/exec"
+
+	"github.com/containerd/containerd"
+	runc "github.com/containerd/go-runc"
+	"github.com/moby/buildkit/executor"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+)
+
+var unsupportedConsoleError = errors.New("tty for runc is only supported on linux")
+
+func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo) (int, error) {
+	if process.Meta.Tty {
+		return 0, unsupportedConsoleError
+	}
+	return w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
+		IO:      &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr},
+		NoPivot: w.noPivot,
+	})
+}
+
+func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) (int, error) {
+	if process.Meta.Tty {
+		return 0, unsupportedConsoleError
+	}
+	err := w.runc.Exec(ctx, id, *specsProcess, &runc.ExecOpts{
+		IO: &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr},
+	})
+
+	var exitError *exec.ExitError
+	if errors.As(err, &exitError) {
+		return exitError.ExitCode(), err
+	}
+	if err != nil {
+		return containerd.UnknownExitStatus, err
+	}
+	return 0, nil
+}