Merge pull request #1741 from coryb/runc-exiterror

update go-runc  module, use runc.ExitError for container exec status

client/build_test.go

@@ -271,9 +271,8 @@ func testClientGatewayContainerCancelOnRelease(t *testing.T, sb integration.Sand
 // We are mimicing: `echo testing | cat | cat > /tmp/foo && cat /tmp/foo`
 func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) {
 	if sb.Rootless() {
-		// TODO fix this
-		// We get `panic: cannot statfs cgroup root` from runc when when running
-		// this test with runc-rootless, no idea why.
+		// TODO remove when https://github.com/opencontainers/runc/pull/2634
+		// is merged and released
 		t.Skip("Skipping oci-rootless for cgroup error")
 	}
 	requiresLinux(t)
@@ -467,9 +466,8 @@ func testClientGatewayContainerPID1Fail(t *testing.T, sb integration.Sandbox) {
 // via `Exec` are shutdown when the primary pid1 process exits
 func testClientGatewayContainerPID1Exit(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
+		// TODO remove when https://github.com/opencontainers/runc/pull/2634
+		// is merged and released
 		t.Skip("Skipping runc-rootless for cgroup error")
 	}
 	requiresLinux(t)
@@ -535,10 +533,9 @@ func testClientGatewayContainerPID1Exit(t *testing.T, sb integration.Sandbox) {
 	}
 
 	_, err = c.Build(ctx, SolveOpt{}, product, b, nil)
-	// pid2 should error with `buildkit-runc did not terminate successfully` on runc or
-	// `exit code: 137` (ie sigkill) on containerd
 	require.Error(t, err)
-	require.Regexp(t, "exit code: 137|buildkit-runc did not terminate successfully", err.Error())
+	// `exit code: 137` (ie sigkill)
+	require.Regexp(t, "exit code: 137", err.Error())
 
 	checkAllReleasable(t, c, sb, true)
 }
@@ -547,9 +544,8 @@ func testClientGatewayContainerPID1Exit(t *testing.T, sb integration.Sandbox) {
 // llb.States
 func testClientGatewayContainerMounts(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
+		// TODO remove when https://github.com/opencontainers/runc/pull/2634
+		// is merged and released
 		t.Skip("Skipping runc-rootless for cgroup error")
 	}
 	requiresLinux(t)
@@ -864,9 +860,8 @@ func (p *testPrompt) wait(msg string) string {
 // 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
+		// TODO remove when https://github.com/opencontainers/runc/pull/2634
+		// is merged and released
 		t.Skip("Skipping runc-rootless for cgroup error")
 	}
 	requiresLinux(t)
@@ -937,12 +932,17 @@ func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) {
 		prompt.SendExpect("ttysize", "100 60")
 		prompt.SendExit(99)
 
-		return &client.Result{}, pid2.Wait()
+		err = pid2.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)
-	require.Regexp(t, "exit code: 99|runc did not terminate successfully", err.Error())
+	require.Regexp(t, "exit code: 99", err.Error())
 
 	inputW.Close()
 	inputR.Close()

executor/runcexecutor/executor.go

@@ -12,6 +12,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/mount"
 	containerdoci "github.com/containerd/containerd/oci"
 	"github.com/containerd/continuity/fs"
@@ -103,15 +104,16 @@ func New(opt Opt, networkProviders map[pb.NetMode]network.Provider) (executor.Ex
 	os.RemoveAll(filepath.Join(root, "resolv.conf"))
 
 	runtime := &runc.Runc{
-		Command:      cmd,
-		Log:          filepath.Join(root, "runc-log.json"),
-		LogFormat:    runc.JSON,
-		PdeathSignal: syscall.SIGKILL, // this can still leak the process
-		Setpgid:      true,
+		Command:   cmd,
+		Log:       filepath.Join(root, "runc-log.json"),
+		LogFormat: runc.JSON,
+		Setpgid:   true,
 		// we don't execute runc with --rootless=(true|false) explicitly,
 		// so as to support non-runc runtimes
 	}
 
+	updateRuncFieldsForHostOS(runtime)
+
 	w := &runcExecutor{
 		runc:             runtime,
 		root:             root,
@@ -324,21 +326,29 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root cache.Mountable,
 		})
 	}
 
-	status, err := w.run(runCtx, id, bundle, process)
+	err = w.run(runCtx, id, bundle, process)
 	close(ended)
+	return exitError(ctx, err)
+}
 
-	if status != 0 || err != nil {
+func exitError(ctx context.Context, err error) error {
+	if err != nil {
 		exitErr := &errdefs.ExitError{
-			ExitCode: uint32(status),
+			ExitCode: containerd.UnknownExitStatus,
 			Err:      err,
 		}
-		err = exitErr
+		var runcExitError *runc.ExitError
+		if errors.As(err, &runcExitError) {
+			exitErr = &errdefs.ExitError{
+				ExitCode: uint32(runcExitError.Status),
+			}
+		}
 		select {
 		case <-ctx.Done():
 			exitErr.Err = errors.Wrapf(ctx.Err(), exitErr.Error())
 			return exitErr
 		default:
-			return stack.Enable(err)
+			return stack.Enable(exitErr)
 		}
 	}
 
@@ -408,14 +418,8 @@ func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.Pro
 		spec.Process.Env = process.Meta.Env
 	}
 
-	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,
-	}
+	err = w.exec(ctx, id, state.Bundle, spec.Process, process)
+	return exitError(ctx, err)
 }
 
 type forwardIO struct {

executor/runcexecutor/executor_common.go

@@ -4,9 +4,7 @@ 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"
@@ -15,30 +13,24 @@ import (
 
 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) {
+func updateRuncFieldsForHostOS(runtime *runc.Runc) {}
+
+func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo) error {
 	if process.Meta.Tty {
-		return 0, unsupportedConsoleError
+		return unsupportedConsoleError
 	}
-	return w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
+	_, err := w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
 		IO:      &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr},
 		NoPivot: w.noPivot,
 	})
+	return err
 }
 
-func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) (int, error) {
+func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) error {
 	if process.Meta.Tty {
-		return 0, unsupportedConsoleError
+		return unsupportedConsoleError
 	}
-	err := w.runc.Exec(ctx, id, *specsProcess, &runc.ExecOpts{
+	return 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
 }

executor/runcexecutor/executor_linux.go

@@ -7,14 +7,12 @@ import (
 	"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"
@@ -24,42 +22,40 @@ import (
 	"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{
+func updateRuncFieldsForHostOS(runtime *runc.Runc) {
+	// PdeathSignal only supported on unix platforms
+	runtime.PdeathSignal = syscall.SIGKILL // this can still leak the process
+}
+
+func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo) error {
+	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) error {
+		_, err := w.runc.Run(ctx, id, bundle, &runc.CreateOpts{
 			NoPivot: w.noPivot,
 			PidFile: pidfile,
 			IO:      io,
 		})
+		return err
 	})
 }
 
-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{
+func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo) error {
+	return w.callWithIO(ctx, id, bundle, process, func(ctx context.Context, pidfile string, io runc.IO) error {
+		return 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)
+type runcCall func(ctx context.Context, pidfile string, io runc.IO) error
 
-func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, process executor.ProcessInfo, call runcCall) (int, error) {
+func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, process executor.ProcessInfo, call runcCall) 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")
+		return errors.Wrap(err, "failed to create pidfile")
 	}
 	defer os.Remove(pidfile.Name())
 	pidfile.Close()
@@ -70,13 +66,13 @@ func (w *runcExecutor) callWithIO(ctx context.Context, id, bundle string, proces
 
 	ptm, ptsName, err := console.NewPty()
 	if err != nil {
-		return containerd.UnknownExitStatus, err
+		return err
 	}
 
 	pts, err := os.OpenFile(ptsName, os.O_RDWR|syscall.O_NOCTTY, 0)
 	if err != nil {
 		ptm.Close()
-		return containerd.UnknownExitStatus, err
+		return err
 	}
 
 	eg, ctx := errgroup.WithContext(ctx)

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/containerd/containerd v1.4.1-0.20200903181227-d4e78200d6da
 	github.com/containerd/continuity v0.0.0-20200710164510-efbc4488d8fe
 	github.com/containerd/go-cni v1.0.1
-	github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328
+	github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34
 	github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9
 	github.com/containerd/typeurl v1.0.1
 	github.com/coreos/go-systemd/v22 v22.1.0

go.sum

@@ -127,6 +127,8 @@ github.com/containerd/go-cni v1.0.1/go.mod h1:+vUpYxKvAF72G9i1WoDOiPGRtQpqsNW/ZH
 github.com/containerd/go-runc v0.0.0-20180907222934-5a6d9f37cfa3/go.mod h1:IV7qH3hrUgRmyYrtgEeGWJfWbgcHL9CSRruz2Vqcph0=
 github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328 h1:PRTagVMbJcCezLcHXe8UJvR1oBzp2lG3CEumeFOLOds=
 github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328/go.mod h1:PpyHrqVs8FTi9vpyHwPwiNEGaACDxT/N/pLcvMSRA9g=
+github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34 h1:jFRg/hwx0DPpcg23MNusAppCDJa5nndN3/RAxSblN58=
+github.com/containerd/go-runc v0.0.0-20201017060547-8c4be61c2e34/go.mod h1:1CDPys/h0SMZoki7dv3bPQ1wJWnmaeZIO026WPts2xM=
 github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9 h1:JEcj3rNCg0Ho5t9kiOa4LV/vaNXbRQ9l2PIRzz2LWCQ=
 github.com/containerd/stargz-snapshotter v0.0.0-20200903042824-2ee75e91f8f9/go.mod h1:f+gZLtYcuNQWxucWyfVEQXSBoGbXNpQ76+XWVMgp+34=
 github.com/containerd/ttrpc v0.0.0-20190828154514-0e0f228740de/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o=