Skip to content

Commit

Permalink
Merge pull request moby#3658 from coryb/gateway-exec-cancel
Browse files Browse the repository at this point in the history
fix gateway exec tty cleanup on context.Canceled
  • Loading branch information
tonistiigi authored Mar 22, 2023
2 parents 2c6ea13 + aa827f5 commit eb5a51e
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 2 deletions.
154 changes: 154 additions & 0 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ func TestClientGatewayIntegration(t *testing.T) {
testClientGatewayContainerPID1Exit,
testClientGatewayContainerMounts,
testClientGatewayContainerPID1Tty,
testClientGatewayContainerCancelPID1Tty,
testClientGatewayContainerExecTty,
testClientGatewayContainerCancelExecTty,
testClientSlowCacheRootfsRef,
testClientGatewayContainerPlatformPATH,
testClientGatewayExecError,
Expand Down Expand Up @@ -923,6 +925,77 @@ func testClientGatewayContainerPID1Tty(t *testing.T, sb integration.Sandbox) {
checkAllReleasable(t, c, sb, true)
}

// testClientGatewayContainerCancelPID1Tty is testing that the tty will cleanly
// shutdown on context cancel
func testClientGatewayContainerCancelPID1Tty(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
ctx := sb.Context()

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, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()

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)
prompt.SendExpect("echo hi", "hi")
cancel()

err = pid1.Wait()
require.ErrorIs(t, err, context.Canceled)

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
Expand Down Expand Up @@ -1071,6 +1144,87 @@ func testClientGatewayContainerExecTty(t *testing.T, sb integration.Sandbox) {
checkAllReleasable(t, c, sb, true)
}

// testClientGatewayContainerExecTty is testing the tty shuts down cleanly
// on context.Cancel
func testClientGatewayContainerCancelExecTty(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
ctx := sb.Context()

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)

execCtx, cancel := context.WithCancel(ctx)
defer cancel()

prompt := newTestPrompt(execCtx, t, inputW, output)
pid2, err := ctr.Start(execCtx, 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)

prompt.SendExpect("echo hi", "hi")
cancel()

err = pid2.Wait()
require.ErrorIs(t, err, context.Canceled)

return &client.Result{}, err
}

_, err = c.Build(ctx, SolveOpt{}, product, b, nil)
require.Error(t, err)
require.Contains(t, err.Error(), context.Canceled.Error())

inputW.Close()
inputR.Close()

checkAllReleasable(t, c, sb, true)
}

func testClientSlowCacheRootfsRef(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)

Expand Down
4 changes: 2 additions & 2 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -928,11 +928,11 @@ func (ctr *container) Start(ctx context.Context, req client.StartRequest) (clien

if msg == nil {
// empty message from ctx cancel, so just start shutting down
// input, but continue processing more exit/done messages
// input
closeDoneOnce.Do(func() {
close(done)
})
continue
return ctx.Err()
}

if file := msg.GetFile(); file != nil {
Expand Down

0 comments on commit eb5a51e

Please sign in to comment.