From e308ef7874d3a2d51d01cc127d9c7f420e83f066 Mon Sep 17 00:00:00 2001 From: Cory Bennett Date: Sun, 11 Oct 2020 19:35:22 +0000 Subject: [PATCH 1/4] add tty support for runc executor Signed-off-by: Cory Bennett --- client/build_test.go | 210 +++++++++++++++++++++++ executor/runcexecutor/executor.go | 32 ++-- executor/runcexecutor/executor_common.go | 44 +++++ executor/runcexecutor/executor_linux.go | 206 ++++++++++++++++++++++ 4 files changed, 470 insertions(+), 22 deletions(-) create mode 100644 executor/runcexecutor/executor_common.go create mode 100644 executor/runcexecutor/executor_linux.go diff --git a/client/build_test.go b/client/build_test.go index 22c9275b..f0bd7596 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -7,6 +7,8 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" + "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,212 @@ 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) { + 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) + + pid1, err := ctr.Start(ctx, client.StartRequest{ + Args: []string{"sh"}, + Tty: true, + Stdin: inputR, + Stdout: &nopCloser{output}, + Stderr: &nopCloser{output}, + Env: []string{"PS1=% "}, + }) + require.NoError(t, err) + sendProcessInput(ctx, t, inputW, pid1) + + 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) + expectProcessOutput(t, output.String()) + + inputW.Close() + inputR.Close() + + checkAllReleasable(t, c, sb, true) +} + +func sendProcessInput(ctx context.Context, t *testing.T, input io.Writer, ctrProc client.ContainerProcess) { + err := ctrProc.Resize(ctx, client.WinSize{ + Rows: 40, + Cols: 80, + }) + require.NoError(t, err) + time.Sleep(500 * time.Millisecond) + + input.Write([]byte("ttysize\n")) + time.Sleep(100 * time.Millisecond) + input.Write([]byte("cd /tmp\n")) + time.Sleep(100 * time.Millisecond) + input.Write([]byte("pwd\n")) + time.Sleep(100 * time.Millisecond) + input.Write([]byte("echo foobar > newfile\n")) + time.Sleep(100 * time.Millisecond) + input.Write([]byte("cat /tmp/newfile\n")) + time.Sleep(500 * time.Millisecond) + + err = ctrProc.Resize(ctx, client.WinSize{ + Rows: 60, + Cols: 100, + }) + require.NoError(t, err) + time.Sleep(500 * time.Millisecond) + + input.Write([]byte("ttysize\n")) + time.Sleep(100 * time.Millisecond) + input.Write([]byte("exit 99\n")) +} + +// badControlRx will allow us to strip out the \r to the CSI "Erase In Display" +// escape sequence +var badControlRx = regexp.MustCompile(`\r.*\x1b\[J`) + +func expectProcessOutput(t *testing.T, output string) { + // filter out stray terminal CSR codes + output = badControlRx.ReplaceAllString(output, "") + // trim out \r so we can cleanly split on newline + output = strings.ReplaceAll(output, "\r", "") + // trim trailing \n + output = strings.TrimSpace(output) + + t.Logf("OUTPUT: %s", output) + outputLines := strings.Split(output, "\n") + + require.Equal(t, []string{ + "% ttysize", + "80 40", + "% cd /tmp", + "% pwd", + "/tmp", + "% echo foobar > newfile", + "% cat /tmp/newfile", + "foobar", + "% ttysize", + "100 60", + "% exit 99", + }, outputLines) +} + +// 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) { + 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) + + pid2, err := ctr.Start(ctx, client.StartRequest{ + Args: []string{"sh"}, + Tty: true, + Stdin: inputR, + Stdout: &nopCloser{output}, + Stderr: &nopCloser{output}, + Env: []string{"PS1=% "}, + }) + require.NoError(t, err) + sendProcessInput(ctx, t, inputW, pid2) + 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()) + expectProcessOutput(t, output.String()) + + inputW.Close() + inputR.Close() + + checkAllReleasable(t, c, sb, true) +} + type nopCloser struct { io.Writer } diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index f7eb7082..fbc7e097 100644 --- a/executor/runcexecutor/executor.go +++ b/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 { diff --git a/executor/runcexecutor/executor_common.go b/executor/runcexecutor/executor_common.go new file mode 100644 index 00000000..729db008 --- /dev/null +++ b/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 +} diff --git a/executor/runcexecutor/executor_linux.go b/executor/runcexecutor/executor_linux.go new file mode 100644 index 00000000..1ebe9ab9 --- /dev/null +++ b/executor/runcexecutor/executor_linux.go @@ -0,0 +1,206 @@ +package runcexecutor + +import ( + "bufio" + "context" + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + "time" + + "github.com/containerd/console" + "github.com/containerd/containerd" + runc "github.com/containerd/go-runc" + "github.com/docker/docker/pkg/signal" + "github.com/moby/buildkit/executor" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/sync/errgroup" +) + +func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo) (int, error) { + return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) (int, error) { + return w.runc.Run(ctx, id, bundle, &runc.CreateOpts{ + NoPivot: w.noPivot, + PidFile: pidfile, + IO: io, + }) + }) +} + +func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) (int, error) { + return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) (int, error) { + err := w.runc.Exec(ctx, id, *specsProcess, &runc.ExecOpts{ + PidFile: pidfile, + IO: io, + }) + var exitError *exec.ExitError + if errors.As(err, &exitError) { + return exitError.ExitCode(), err + } + if err != nil { + return containerd.UnknownExitStatus, err + } + return 0, nil + }) +} + +type runcCall func(ctx context.Context, pidfile string, io runc.IO) (int, error) + +func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, process executor.ProcessInfo, call runcCall) (int, error) { + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + pidfile, err := ioutil.TempFile(bundle, "*.pid") + if err != nil { + return containerd.UnknownExitStatus, errors.Wrap(err, "failed to create pidfile") + } + defer os.Remove(pidfile.Name()) + pidfile.Close() + + if !process.Meta.Tty { + return call(ctx, pidfile.Name(), &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr}) + } + + ptm, ptsName, err := console.NewPty() + if err != nil { + return containerd.UnknownExitStatus, err + } + + pts, err := os.OpenFile(ptsName, os.O_RDWR|syscall.O_NOCTTY, 0) + if err != nil { + ptm.Close() + return containerd.UnknownExitStatus, err + } + + eg, ctx := errgroup.WithContext(ctx) + + defer func() { + if process.Stdin != nil { + process.Stdin.Close() + } + pts.Close() + ptm.Close() + cancel() // this will shutdown resize loop + err := eg.Wait() + if err != nil { + logrus.Warningf("error while shutting down tty io: %s", err) + } + }() + + if process.Stdin != nil { + eg.Go(func() error { + _, err := io.Copy(ptm, process.Stdin) + // stdin might be a pipe, so this is like EOF + if errors.Is(err, io.ErrClosedPipe) { + return nil + } + return err + }) + } + + if process.Stdout != nil { + eg.Go(func() error { + _, err := io.Copy(process.Stdout, ptm) + // ignore `read /dev/ptmx: input/output error` when ptm is closed + var ptmClosedError *os.PathError + if errors.As(err, &ptmClosedError) { + if ptmClosedError.Op == "read" && + ptmClosedError.Path == "/dev/ptmx" && + ptmClosedError.Err == syscall.Errno(0x5) { + return nil + } + } + return err + }) + } + + eg.Go(func() error { + // need to poll until the pidfile has the pid written to it + pidfileCtx, timeout := context.WithTimeout(ctx, 10*time.Second) + defer timeout() + + var runcProcess *os.Process + for { + st, err := os.Stat(pidfile.Name()) + if err == nil && st.Size() > 0 { + pid, err := runc.ReadPidFile(pidfile.Name()) + if err != nil { + return errors.Wrapf(err, "unable to read pid file: %s", pidfile.Name()) + } + // pid will be for the process in process.Meta, not the parent runc process. + // We need to send SIGWINCH to the runc process, not the process.Meta process. + ppid, err := getppid(pid) + if err != nil { + return errors.Wrapf(err, "unable to find runc process (parent of %d)", pid) + } + runcProcess, err = os.FindProcess(ppid) + if err != nil { + return errors.Wrapf(err, "unable to find process for pid %d", ppid) + } + break + } + select { + case <-pidfileCtx.Done(): + return errors.New("pidfile never updated") + case <-time.After(100 * time.Microsecond): + } + } + for { + select { + case <-ctx.Done(): + return nil + case resize := <-process.Resize: + err = ptm.Resize(console.WinSize{ + Height: uint16(resize.Rows), + Width: uint16(resize.Cols), + }) + if err != nil { + logrus.Errorf("failed to resize ptm: %s", err) + } + err = runcProcess.Signal(signal.SIGWINCH) + if err != nil { + logrus.Errorf("failed to send SIGWINCH to process: %s", err) + } + } + } + }) + + runcIO := &forwardIO{} + if process.Stdin != nil { + runcIO.stdin = pts + } + if process.Stdout != nil { + runcIO.stdout = pts + } + if process.Stderr != nil { + runcIO.stderr = pts + } + + return call(ctx, pidfile.Name(), runcIO) +} + +const PPidStatusPrefix = "PPid:\t" + +func getppid(pid int) (int, error) { + fh, err := os.Open(fmt.Sprintf("/proc/%d/status", pid)) + if err != nil { + return -1, err + } + + defer fh.Close() + scanner := bufio.NewScanner(fh) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, PPidStatusPrefix) { + return strconv.Atoi(strings.TrimPrefix(line, PPidStatusPrefix)) + } + } + return -1, errors.Errorf("PPid line not found in /proc/%d/status", pid) +} From 4d265f5d22d30e67f01e6173e4e5520a2c70c749 Mon Sep 17 00:00:00 2001 From: Cory Bennett Date: Sat, 17 Oct 2020 06:52:06 +0000 Subject: [PATCH 2/4] refactor tty output tests to avoid flakey sleeps Signed-off-by: Cory Bennett --- client/build_test.go | 144 +++++++++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 61 deletions(-) diff --git a/client/build_test.go b/client/build_test.go index f0bd7596..93edb22f 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -3,11 +3,11 @@ package client import ( "bytes" "context" + "fmt" "io" "io/ioutil" "os" "path/filepath" - "regexp" "strings" "testing" "time" @@ -738,6 +738,9 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) { 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) @@ -762,16 +765,27 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) { 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{"PS1=% "}, + Env: []string{fmt.Sprintf("PS1=%s", prompt.String())}, }) require.NoError(t, err) - sendProcessInput(ctx, t, inputW, pid1) + 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 @@ -783,7 +797,6 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) { _, err = c.Build(ctx, SolveOpt{}, product, b, nil) require.Error(t, err) - expectProcessOutput(t, output.String()) inputW.Close() inputR.Close() @@ -791,65 +804,60 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) { checkAllReleasable(t, c, sb, true) } -func sendProcessInput(ctx context.Context, t *testing.T, input io.Writer, ctrProc client.ContainerProcess) { - err := ctrProc.Resize(ctx, client.WinSize{ - Rows: 40, - Cols: 80, - }) - require.NoError(t, err) - time.Sleep(500 * time.Millisecond) - - input.Write([]byte("ttysize\n")) - time.Sleep(100 * time.Millisecond) - input.Write([]byte("cd /tmp\n")) - time.Sleep(100 * time.Millisecond) - input.Write([]byte("pwd\n")) - time.Sleep(100 * time.Millisecond) - input.Write([]byte("echo foobar > newfile\n")) - time.Sleep(100 * time.Millisecond) - input.Write([]byte("cat /tmp/newfile\n")) - time.Sleep(500 * time.Millisecond) - - err = ctrProc.Resize(ctx, client.WinSize{ - Rows: 60, - Cols: 100, - }) - require.NoError(t, err) - time.Sleep(500 * time.Millisecond) - - input.Write([]byte("ttysize\n")) - time.Sleep(100 * time.Millisecond) - input.Write([]byte("exit 99\n")) +type testPrompt struct { + ctx context.Context + t *testing.T + output *bytes.Buffer + input io.Writer + prompt string + pos int } -// badControlRx will allow us to strip out the \r to the CSI "Erase In Display" -// escape sequence -var badControlRx = regexp.MustCompile(`\r.*\x1b\[J`) +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 expectProcessOutput(t *testing.T, output string) { - // filter out stray terminal CSR codes - output = badControlRx.ReplaceAllString(output, "") - // trim out \r so we can cleanly split on newline - output = strings.ReplaceAll(output, "\r", "") - // trim trailing \n - output = strings.TrimSpace(output) +func (p *testPrompt) String() string { return p.prompt } - t.Logf("OUTPUT: %s", output) - outputLines := strings.Split(output, "\n") +func (p *testPrompt) SendExit(status int) { + p.input.Write([]byte(fmt.Sprintf("exit %d\n", status))) +} - require.Equal(t, []string{ - "% ttysize", - "80 40", - "% cd /tmp", - "% pwd", - "/tmp", - "% echo foobar > newfile", - "% cat /tmp/newfile", - "foobar", - "% ttysize", - "100 60", - "% exit 99", - }, outputLines) +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 @@ -873,6 +881,8 @@ func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) { 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) @@ -904,23 +914,35 @@ func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) { 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{"PS1=% "}, + Env: []string{fmt.Sprintf("PS1=%s", prompt.String())}, }) require.NoError(t, err) - sendProcessInput(ctx, t, inputW, pid2) + + 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()) - expectProcessOutput(t, output.String()) inputW.Close() inputR.Close() From cb12a5642faea1648e582153dfa492c065d7baf6 Mon Sep 17 00:00:00 2001 From: Cory Bennett Date: Sat, 17 Oct 2020 06:53:25 +0000 Subject: [PATCH 3/4] shutdown container process when context is done Signed-off-by: Cory Bennett --- frontend/gateway/grpcclient/client.go | 30 ++++++++++++++++++++------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/frontend/gateway/grpcclient/client.go b/frontend/gateway/grpcclient/client.go index c6035961..92e416cb 100644 --- a/frontend/gateway/grpcclient/client.go +++ b/frontend/gateway/grpcclient/client.go @@ -475,14 +475,15 @@ func (b *procMessageForwarder) Send(ctx context.Context, m *pb.ExecMessage) { } } -func (b *procMessageForwarder) Recv(ctx context.Context) *pb.ExecMessage { +func (b *procMessageForwarder) Recv(ctx context.Context) (m *pb.ExecMessage, ok bool) { select { case <-ctx.Done(): + return nil, true case <-b.done: - case m := <-b.msgs: - return m + return nil, false + case m = <-b.msgs: + return m, true } - return nil } func (b *procMessageForwarder) Close() { @@ -734,7 +735,7 @@ func (ctr *container) Start(ctx context.Context, req client.StartRequest) (clien return nil, err } - msg := msgs.Recv(ctx) + msg, _ := msgs.Recv(ctx) if msg == nil { return nil, errors.Errorf("failed to receive started message") } @@ -798,13 +799,24 @@ func (ctr *container) Start(ctx context.Context, req client.StartRequest) (clien } ctrProc.eg.Go(func() error { + var closeDoneOnce sync.Once var exitError error for { - msg := msgs.Recv(ctx) - if msg == nil { + msg, ok := msgs.Recv(ctx) + if !ok { + // no more messages, return return exitError } + if msg == nil { + // empty message from ctx cancel, so just start shutting down + // input, but continue processing more exit/done messages + closeDoneOnce.Do(func() { + close(done) + }) + continue + } + if file := msg.GetFile(); file != nil { var out io.WriteCloser switch file.Fd { @@ -826,7 +838,9 @@ func (ctr *container) Start(ctx context.Context, req client.StartRequest) (clien } else if exit := msg.GetExit(); exit != nil { // capture exit message to exitError so we can return it after // the server sends the Done message - close(done) + closeDoneOnce.Do(func() { + close(done) + }) if exit.Code == 0 { continue } From 74db85d357433e82eee930c159a5cd2186c78423 Mon Sep 17 00:00:00 2001 From: Cory Bennett Date: Mon, 19 Oct 2020 21:28:39 +0000 Subject: [PATCH 4/4] syscall.Errno(0x5) == syscall.EIO Signed-off-by: Cory Bennett --- executor/runcexecutor/executor_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/runcexecutor/executor_linux.go b/executor/runcexecutor/executor_linux.go index 1ebe9ab9..0daa8d23 100644 --- a/executor/runcexecutor/executor_linux.go +++ b/executor/runcexecutor/executor_linux.go @@ -113,7 +113,7 @@ func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, proces if errors.As(err, &ptmClosedError) { if ptmClosedError.Op == "read" && ptmClosedError.Path == "/dev/ptmx" && - ptmClosedError.Err == syscall.Errno(0x5) { + ptmClosedError.Err == syscall.EIO { return nil } }