From e31fc48a5474ef558cd93dd5e0cbbb23005a12a7 Mon Sep 17 00:00:00 2001 From: Justin Chadwell <me@jedevc.com> Date: Wed, 13 Dec 2023 17:43:36 +0000 Subject: [PATCH] chore: remove WithFailFast option WithFailFast was not actually causing any change in behavior, so we can remove the option completely. The option set FailOnNonTempDialError(true) on the gRPC connection. However, the help text for this method option declares that it "does not do anything useful unless you are also using WithBlock()" - which we do not do. I've verified that the only client-side code path that actually handles this option is under a check for if WithBlock() was also used. We can remove this option instead of fixing it, since the need for this functionality has been replaced by the more buildkit-native client.Wait function - this function also correctly waits for the buildkit server to begin running, and to start serving requests (which is generally the desired behaviour). If users actually desire this option (which was not fully working previously), they can use the WithGRPCDialOption, and pass FailOnNonTempDialError(true) directly, alongside WithBlock. Signed-off-by: Justin Chadwell <me@jedevc.com> --- client/client.go | 11 ----------- cmd/buildctl/common/common.go | 4 +--- examples/build-using-dockerfile/main.go | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/client/client.go b/client/client.go index ea8b0e0e0b65..1a11aa2409d6 100644 --- a/client/client.go +++ b/client/client.go @@ -59,9 +59,6 @@ func New(ctx context.Context, address string, opts ...ClientOpt) (*Client, error var creds *withCredentials for _, o := range opts { - if _, ok := o.(*withFailFast); ok { - gopts = append(gopts, grpc.FailOnNonTempDialError(true)) - } if credInfo, ok := o.(*withCredentials); ok { if creds == nil { creds = &withCredentials{} @@ -216,14 +213,6 @@ func (c *Client) Close() error { return c.conn.Close() } -type withFailFast struct{} - -func (*withFailFast) isClientOpt() {} - -func WithFailFast() ClientOpt { - return &withFailFast{} -} - type withDialer struct { dialer func(context.Context, string) (net.Conn, error) } diff --git a/cmd/buildctl/common/common.go b/cmd/buildctl/common/common.go index 2f32b71f0da5..f8f5fa1223a0 100644 --- a/cmd/buildctl/common/common.go +++ b/cmd/buildctl/common/common.go @@ -65,10 +65,8 @@ func ResolveClient(c *cli.Context) (*client.Client, error) { key = c.GlobalString("tlskey") } - opts := []client.ClientOpt{client.WithFailFast()} - ctx := CommandContext(c) - + var opts []client.ClientOpt if span := trace.SpanFromContext(ctx); span.SpanContext().IsValid() { opts = append(opts, client.WithTracerProvider(span.TracerProvider())) diff --git a/examples/build-using-dockerfile/main.go b/examples/build-using-dockerfile/main.go index 0368ee8e5605..8acfc4e768c2 100644 --- a/examples/build-using-dockerfile/main.go +++ b/examples/build-using-dockerfile/main.go @@ -80,7 +80,7 @@ func action(clicontext *cli.Context) error { if tag := clicontext.String("tag"); tag == "" { return errors.New("tag is not specified") } - c, err := client.New(ctx, clicontext.String("buildkit-addr"), client.WithFailFast()) + c, err := client.New(ctx, clicontext.String("buildkit-addr")) if err != nil { return err }