From 97ffd6cce8fed8de7d4d86feef7f83e517a3275f Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Wed, 6 Jun 2018 11:52:53 +0100 Subject: [PATCH] client: take a context.Context to New(). This allows two things: - The caller to set a shorter timeout than previously hardcoded 30s. In `buildctl` reduce the timeout to 5s. Since the existing timeout has gone callers will need to arrange to pass one themselves. - The caller can arrange for the context to be cancelled for other reasons, use this in `buildctl` to plumb through the Ctrl-C handling, meaning that `buildctl` now exits almost immediately on Ctrl-C instead of after several seconds. Signed-off-by: Ian Campbell --- client/client.go | 6 +-- client/client_test.go | 38 +++++++++--------- cmd/buildctl/debug/workers.go | 2 +- cmd/buildctl/main.go | 18 ++++++++- examples/build-using-dockerfile/main.go | 6 ++- frontend/dockerfile/dockerfile_test.go | 51 +++++++++++++------------ 6 files changed, 67 insertions(+), 54 deletions(-) diff --git a/client/client.go b/client/client.go index c71c00c0e..403ed52a8 100644 --- a/client/client.go +++ b/client/client.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "crypto/x509" "io/ioutil" - "time" "github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc" controlapi "github.com/moby/buildkit/api/services/control" @@ -23,7 +22,7 @@ type Client struct { type ClientOpt interface{} // New returns a new buildkit client. Address can be empty for the system-default address. -func New(address string, opts ...ClientOpt) (*Client, error) { +func New(ctx context.Context, address string, opts ...ClientOpt) (*Client, error) { gopts := []grpc.DialOption{ grpc.WithDialer(dialer), grpc.FailOnNonTempDialError(true), @@ -54,9 +53,6 @@ func New(address string, opts ...ClientOpt) (*Client, error) { address = appdefaults.Address } - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - conn, err := grpc.DialContext(ctx, address, gopts...) if err != nil { return nil, errors.Wrapf(err, "failed to dial %q . make sure buildkitd is running", address) diff --git a/client/client_test.go b/client/client_test.go index e4b66d892..a6edc26db 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -59,7 +59,7 @@ func TestClientIntegration(t *testing.T) { func testTmpfsMounts(t *testing.T, sb integration.Sandbox) { t.Parallel() requiresLinux(t) - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -76,7 +76,7 @@ func testTmpfsMounts(t *testing.T, sb integration.Sandbox) { func testLocalSymlinkEscape(t *testing.T, sb integration.Sandbox) { t.Parallel() requiresLinux(t) - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -134,7 +134,7 @@ func testLocalSymlinkEscape(t *testing.T, sb integration.Sandbox) { func testRelativeWorkDir(t *testing.T, sb integration.Sandbox) { t.Parallel() requiresLinux(t) - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -164,7 +164,7 @@ func testRelativeWorkDir(t *testing.T, sb integration.Sandbox) { func testCallDiskUsage(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() _, err = c.DiskUsage(context.TODO()) @@ -174,7 +174,7 @@ func testCallDiskUsage(t *testing.T, sb integration.Sandbox) { func testBuildMultiMount(t *testing.T, sb integration.Sandbox) { t.Parallel() requiresLinux(t) - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -196,7 +196,7 @@ func testBuildMultiMount(t *testing.T, sb integration.Sandbox) { func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -284,7 +284,7 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) { func testResolveAndHosts(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -324,7 +324,7 @@ func testResolveAndHosts(t *testing.T, sb integration.Sandbox) { func testUser(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -377,7 +377,7 @@ func testUser(t *testing.T, sb integration.Sandbox) { func testOCIExporter(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -476,7 +476,7 @@ func testOCIExporter(t *testing.T, sb integration.Sandbox) { func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -673,7 +673,7 @@ func testBasicCacheImportExport(t *testing.T, sb integration.Sandbox) { } require.NoError(t, err) - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -738,7 +738,7 @@ func testBasicCacheImportExport(t *testing.T, sb integration.Sandbox) { func testCachedMounts(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -801,7 +801,7 @@ func testCachedMounts(t *testing.T, sb integration.Sandbox) { func testDuplicateWhiteouts(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -869,7 +869,7 @@ func testDuplicateWhiteouts(t *testing.T, sb integration.Sandbox) { func testWhiteoutParentDir(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -931,7 +931,7 @@ func testWhiteoutParentDir(t *testing.T, sb integration.Sandbox) { // #296 func testSchema1Image(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -949,7 +949,7 @@ func testSchema1Image(t *testing.T, sb integration.Sandbox) { // #319 func testMountWithNoSource(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -979,7 +979,7 @@ func testMountWithNoSource(t *testing.T, sb integration.Sandbox) { // #324 func testReadonlyRootFS(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1008,7 +1008,7 @@ func testReadonlyRootFS(t *testing.T, sb integration.Sandbox) { func testProxyEnv(t *testing.T, sb integration.Sandbox) { t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1208,7 +1208,7 @@ loop0: func testInvalidExporter(t *testing.T, sb integration.Sandbox) { requiresLinux(t) t.Parallel() - c, err := New(sb.Address()) + c, err := New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() diff --git a/cmd/buildctl/debug/workers.go b/cmd/buildctl/debug/workers.go index e59e306ad..0b4668c84 100644 --- a/cmd/buildctl/debug/workers.go +++ b/cmd/buildctl/debug/workers.go @@ -28,7 +28,7 @@ var WorkersCommand = cli.Command{ } func resolveClient(c *cli.Context) (*client.Client, error) { - return client.New(c.GlobalString("addr"), client.WithBlock()) + return client.New(commandContext(c), c.GlobalString("addr"), client.WithBlock()) } func listWorkers(clicontext *cli.Context) error { diff --git a/cmd/buildctl/main.go b/cmd/buildctl/main.go index 95e02382b..a5e23b27b 100644 --- a/cmd/buildctl/main.go +++ b/cmd/buildctl/main.go @@ -1,9 +1,11 @@ package main import ( + "context" "fmt" "net/url" "os" + "time" "github.com/moby/buildkit/client" "github.com/moby/buildkit/util/appdefaults" @@ -57,6 +59,11 @@ func main() { Usage: "client key", Value: "", }, + cli.IntFlag{ + Name: "timeout", + Usage: "timeout backend connection after value seconds", + Value: 5, + }, } app.Commands = []cli.Command{ @@ -106,12 +113,19 @@ func resolveClient(c *cli.Context) (*client.Client, error) { opts := []client.ClientOpt{client.WithBlock()} - if span := opentracing.SpanFromContext(commandContext(c)); span != nil { + ctx := commandContext(c) + + if span := opentracing.SpanFromContext(ctx); span != nil { opts = append(opts, client.WithTracer(span.Tracer())) } if caCert != "" || cert != "" || key != "" { opts = append(opts, client.WithCredentials(serverName, caCert, cert, key)) } - return client.New(c.GlobalString("addr"), opts...) + + timeout := time.Duration(c.GlobalInt("timeout")) + ctx, cancel := context.WithTimeout(ctx, timeout*time.Second) + defer cancel() + + return client.New(ctx, c.GlobalString("addr"), opts...) } diff --git a/examples/build-using-dockerfile/main.go b/examples/build-using-dockerfile/main.go index 08143a8e2..0d2ae7936 100644 --- a/examples/build-using-dockerfile/main.go +++ b/examples/build-using-dockerfile/main.go @@ -66,10 +66,12 @@ By default, the built image is loaded to Docker. } func action(clicontext *cli.Context) error { + ctx := appcontext.Context() + if tag := clicontext.String("tag"); tag == "" { return errors.New("tag is not specified") } - c, err := client.New(clicontext.String("buildkit-addr"), client.WithBlock()) + c, err := client.New(ctx, clicontext.String("buildkit-addr"), client.WithBlock()) if err != nil { return err } @@ -79,7 +81,7 @@ func action(clicontext *cli.Context) error { return err } ch := make(chan *client.SolveStatus) - eg, ctx := errgroup.WithContext(appcontext.Context()) + eg, ctx := errgroup.WithContext(ctx) eg.Go(func() error { _, err := c.Solve(ctx, nil, *solveOpt, ch) return err diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 1bb03f2c3..271eb39d3 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -93,7 +93,7 @@ COPY sub/l* alllinks/ require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -138,7 +138,7 @@ COPY --from=0 /foo /foo require.NoError(t, err) defer os.RemoveAll(destDir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -182,7 +182,7 @@ CMD ["test"] require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -272,7 +272,7 @@ LABEL foo=bar require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -384,7 +384,7 @@ FROM busybox:${tag} require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -872,7 +872,7 @@ EXPOSE 5000 require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -958,7 +958,7 @@ Dockerfile require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1017,12 +1017,13 @@ COPY . . require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + c, err := client.New(ctx, sb.Address()) require.NoError(t, err) defer c.Close() - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() _, err = c.Solve(ctx, nil, client.SolveOpt{ Frontend: "dockerfile.v0", LocalDirs: map[string]string{ @@ -1133,7 +1134,7 @@ USER nobody require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1229,7 +1230,7 @@ COPY --from=base /out / require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1281,7 +1282,7 @@ COPY files dest require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1326,7 +1327,7 @@ COPY $FOO baz require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1378,7 +1379,7 @@ COPY sub/dir1 subdest6 require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1486,7 +1487,7 @@ COPY --from=build foo bar2 require.NoError(t, err) defer os.RemoveAll(destDir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1572,7 +1573,7 @@ COPY foo bar require.NoError(t, err) defer os.RemoveAll(destDir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1606,7 +1607,7 @@ COPY --from=busybox /etc/passwd test require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1683,7 +1684,7 @@ COPY --from=stage1 baz bax require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1723,7 +1724,7 @@ LABEL foo=bar require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1811,7 +1812,7 @@ COPY --from=base unique / require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1892,7 +1893,7 @@ RUN echo bar > bar require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -1978,7 +1979,7 @@ RUN echo bar > bar require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -2054,7 +2055,7 @@ COPY --from=s1 unique2 / require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close() @@ -2140,7 +2141,7 @@ COPY --from=build /out / require.NoError(t, err) defer os.RemoveAll(dir) - c, err := client.New(sb.Address()) + c, err := client.New(context.TODO(), sb.Address()) require.NoError(t, err) defer c.Close()