From f216b71ad29da639cd51d2ddb8429276accbdeb3 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 17:39:22 -0800 Subject: [PATCH 1/5] lint: enable gosimple Signed-off-by: Tonis Tiigi --- .golangci.yml | 1 + commands/ls.go | 4 ++-- controller/pb/progress.go | 4 +--- util/dockerutil/progress.go | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 89d00e25..a9ccf98a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,6 +6,7 @@ run: linters: enable: - gofmt + - gosimple - govet - depguard - goimports diff --git a/commands/ls.go b/commands/ls.go index 33a84f29..f5295ac0 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -319,7 +319,7 @@ func (tp truncatedPlatforms) String() string { if tpf, ok := tp.res[mpf]; ok { seen[mpf] = struct{}{} if len(tpf) == 1 { - out = append(out, fmt.Sprintf("%s", tpf[0])) + out = append(out, tpf[0]) count++ } else { hasPreferredPlatform := false @@ -347,7 +347,7 @@ func (tp truncatedPlatforms) String() string { continue } if len(tp.res[mpf]) == 1 { - out = append(out, fmt.Sprintf("%s", tp.res[mpf][0])) + out = append(out, tp.res[mpf][0]) count++ } else { hasPreferredPlatform := false diff --git a/controller/pb/progress.go b/controller/pb/progress.go index 5883cfca..dd68e7b4 100644 --- a/controller/pb/progress.go +++ b/controller/pb/progress.go @@ -22,9 +22,7 @@ func (w *writer) Write(status *client.SolveStatus) { w.ch <- ToControlStatus(status) } -func (w *writer) WriteBuildRef(target string, ref string) { - return -} +func (w *writer) WriteBuildRef(target string, ref string) {} func (w *writer) ValidateLogSource(digest.Digest, interface{}) bool { return true diff --git a/util/dockerutil/progress.go b/util/dockerutil/progress.go index 62294309..352d1fce 100644 --- a/util/dockerutil/progress.go +++ b/util/dockerutil/progress.go @@ -55,7 +55,7 @@ func fromReader(l progress.SubLogger, rc io.ReadCloser) error { Started: &now, } } - timeDelta := time.Now().Sub(st.Timestamp) + timeDelta := time.Since(st.Timestamp) if timeDelta < minTimeDelta { continue } From 0c629335ac4ba3640d75400e10e8134d8a1b558d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 17:40:42 -0800 Subject: [PATCH 2/5] lint: sort linters Signed-off-by: Tonis Tiigi --- .golangci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a9ccf98a..6151fb5e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,20 +5,20 @@ run: linters: enable: + - depguard + - forbidigo - gofmt + - goimports + - gosec - gosimple - govet - - depguard - - goimports - ineffassign - misspell - - unused + - nolintlint - revive - staticcheck - typecheck - - nolintlint - - gosec - - forbidigo + - unused disable-all: true linters-settings: From c0fd64f4f87fd0046090cab9a1359353ad30d8e6 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 17:51:24 -0800 Subject: [PATCH 3/5] lint: enable linters from buildkit Skipping errname and testifylint Signed-off-by: Tonis Tiigi --- .golangci.yml | 12 ++++++++++++ bake/compose.go | 1 - bake/hclparser/stdlib.go | 1 - builder/builder.go | 18 +++++++++--------- commands/build.go | 1 - commands/imagetools/create.go | 2 +- commands/use.go | 1 - controller/pb/path.go | 1 - controller/remote/io.go | 1 - driver/docker-container/driver.go | 1 - driver/docker/version.go | 5 ----- tests/bake.go | 3 +-- tests/build.go | 1 - util/buildflags/export.go | 1 - util/imagetools/imagetools_helpers_test.go | 1 - 15 files changed, 23 insertions(+), 27 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6151fb5e..a43f7f3c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,23 +5,35 @@ run: linters: enable: + - bodyclose - depguard - forbidigo + - gocritic - gofmt - goimports - gosec - gosimple - govet - ineffassign + - makezero - misspell + - noctx - nolintlint - revive - staticcheck - typecheck - unused + - whitespace disable-all: true linters-settings: + gocritic: + disabled-checks: + - "ifElseChain" + - "assignOp" + - "appendAssign" + - "singleCaseSwitch" + - "exitAfterDefer" # FIXME govet: enable: - nilness diff --git a/bake/compose.go b/bake/compose.go index d1217338..d72dbe78 100644 --- a/bake/compose.go +++ b/bake/compose.go @@ -179,7 +179,6 @@ func ParseCompose(cfgs []composetypes.ConfigFile, envs map[string]string) (*Conf c.Targets = append(c.Targets, t) } c.Groups = append(c.Groups, g) - } return &c, nil diff --git a/bake/hclparser/stdlib.go b/bake/hclparser/stdlib.go index b0e094f6..df04bb8a 100644 --- a/bake/hclparser/stdlib.go +++ b/bake/hclparser/stdlib.go @@ -170,7 +170,6 @@ func indexOfFunc() function.Function { } } return cty.NilVal, errors.New("item not found") - }, }) } diff --git a/builder/builder.go b/builder/builder.go index dfa5051f..fb54886d 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -288,7 +288,15 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) { return nil, err } - builders := make([]*Builder, len(storeng)) + contexts, err := dockerCli.ContextStore().List() + if err != nil { + return nil, err + } + sort.Slice(contexts, func(i, j int) bool { + return contexts[i].Name < contexts[j].Name + }) + + builders := make([]*Builder, len(storeng), len(storeng)+len(contexts)) seen := make(map[string]struct{}) for i, ng := range storeng { b, err := New(dockerCli, @@ -303,14 +311,6 @@ func GetBuilders(dockerCli command.Cli, txn *store.Txn) ([]*Builder, error) { seen[b.NodeGroup.Name] = struct{}{} } - contexts, err := dockerCli.ContextStore().List() - if err != nil { - return nil, err - } - sort.Slice(contexts, func(i, j int) bool { - return contexts[i].Name < contexts[j].Name - }) - for _, c := range contexts { // if a context has the same name as an instance from the store, do not // add it to the builders list. An instance from the store takes diff --git a/commands/build.go b/commands/build.go index 1904a02d..e6fdb2f5 100644 --- a/commands/build.go +++ b/commands/build.go @@ -885,7 +885,6 @@ func printWarnings(w io.Writer, warnings []client.VertexWarning, mode progressui src.Print(w) } fmt.Fprintf(w, "\n") - } } diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index 05f460d7..81471810 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -42,7 +42,7 @@ func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, arg return errors.Errorf("can't push with no tags specified, please set --tag or --dry-run") } - fileArgs := make([]string, len(in.files)) + fileArgs := make([]string, len(in.files), len(in.files)+len(args)) for i, f := range in.files { dt, err := os.ReadFile(f) if err != nil { diff --git a/commands/use.go b/commands/use.go index 30877a8a..58442281 100644 --- a/commands/use.go +++ b/commands/use.go @@ -46,7 +46,6 @@ func runUse(dockerCli command.Cli, in useOptions) error { return errors.Errorf("run `docker context use %s` to switch to context %s", in.builder, in.builder) } } - } return errors.Wrapf(err, "failed to find instance %q", in.builder) } diff --git a/controller/pb/path.go b/controller/pb/path.go index b04b355d..b793a2de 100644 --- a/controller/pb/path.go +++ b/controller/pb/path.go @@ -153,7 +153,6 @@ func ResolveOptionPaths(options *BuildOptions) (_ *BuildOptions, err error) { } } ps = append(ps, p) - } s.Paths = ps ssh = append(ssh, s) diff --git a/controller/remote/io.go b/controller/remote/io.go index df28b445..9a29534f 100644 --- a/controller/remote/io.go +++ b/controller/remote/io.go @@ -302,7 +302,6 @@ func attachIO(ctx context.Context, stream msgStream, initMessage *pb.InitMessage out = cfg.stderr default: return errors.Errorf("unsupported fd %d", file.Fd) - } if out == nil { logrus.Warnf("attachIO: no writer for fd %d", file.Fd) diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 2eede55b..b0ac0720 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -177,7 +177,6 @@ func (d *Driver) create(ctx context.Context, l progress.SubLogger) error { break } } - } _, err := d.DockerAPI.ContainerCreate(ctx, cfg, hc, &network.NetworkingConfig{}, nil, d.Name) if err != nil && !errdefs.IsConflict(err) { diff --git a/driver/docker/version.go b/driver/docker/version.go index 2de359de..e71608d1 100644 --- a/driver/docker/version.go +++ b/driver/docker/version.go @@ -176,11 +176,6 @@ func resolveBuildKitVersion(ver string) (string, error) { if err != nil { return "", err } - //if _, errs := c.Validate(mobyVersion); len(errs) > 0 { - // for _, err := range errs { - // fmt.Printf("%s: %v\n", m.MobyVersionConstraint, err) - // } - //} if !c.Check(mobyVersion) { continue } diff --git a/tests/bake.go b/tests/bake.go index 3f991d03..c089e732 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -978,7 +978,6 @@ func testBakeMultiPlatform(t *testing.T, sb integration.Sandbox) { require.NotNil(t, img) img = imgs.Find("linux/arm64") require.NotNil(t, img) - } else { require.Error(t, err, string(out)) require.Contains(t, string(out), "Multi-platform build is not supported") @@ -1468,7 +1467,7 @@ target "third" { fstest.CreateFile("docker-bake.hcl", bakefile, 0600), ) - dockerfilePathFirst := filepath.Join("Dockerfile") + dockerfilePathFirst := "Dockerfile" dockerfilePathSecond := filepath.Join("subdir", "Dockerfile") dockerfilePathThird := filepath.Join("subdir", "subsubdir", "Dockerfile") diff --git a/tests/build.go b/tests/build.go index 046689dd..bbdc768f 100644 --- a/tests/build.go +++ b/tests/build.go @@ -649,7 +649,6 @@ func testBuildMultiPlatform(t *testing.T, sb integration.Sandbox) { require.NotNil(t, img) img = imgs.Find("linux/arm64") require.NotNil(t, img) - } else { require.Error(t, err, string(out)) require.Contains(t, string(out), "Multi-platform build is not supported") diff --git a/util/buildflags/export.go b/util/buildflags/export.go index 37f3c274..60c561f4 100644 --- a/util/buildflags/export.go +++ b/util/buildflags/export.go @@ -138,7 +138,6 @@ func ParseAnnotations(inp []string) (map[exptypes.AnnotationKey]string, error) { } annotations[ak] = v } - } return annotations, nil } diff --git a/util/imagetools/imagetools_helpers_test.go b/util/imagetools/imagetools_helpers_test.go index 77a3d74b..44490683 100644 --- a/util/imagetools/imagetools_helpers_test.go +++ b/util/imagetools/imagetools_helpers_test.go @@ -47,7 +47,6 @@ func (f mockFetcher) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.Rea reader := io.NopCloser(strings.NewReader(desc.Annotations["test_content"])) return reader, nil } - } func (r mockResolver) Resolve(ctx context.Context, ref string) (name string, desc ocispec.Descriptor, err error) { From e7a53fb829eb927b94786a2a27692891900b58c9 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 18:21:44 -0800 Subject: [PATCH 4/5] lint: enable forbidigo context rules Signed-off-by: Tonis Tiigi --- .golangci.yml | 4 ++++ build/invoke.go | 12 ++++++------ build/result.go | 4 ++-- builder/builder.go | 5 +++-- commands/bake.go | 4 ++-- commands/build.go | 4 ++-- commands/imagetools/create.go | 4 ++-- commands/inspect.go | 6 ++++-- commands/ls.go | 6 ++++-- commands/rm.go | 5 +++-- controller/local/controller.go | 4 ++-- controller/processes/processes.go | 17 ++++++++++++----- controller/remote/controller.go | 10 ++++++---- controller/remote/io.go | 2 +- controller/remote/server.go | 18 +++++++++--------- driver/docker-container/driver.go | 2 +- driver/kubernetes/driver.go | 2 +- driver/remote/driver.go | 5 +++-- monitor/monitor.go | 4 ++-- util/dockerutil/client.go | 15 ++++++--------- util/gitutil/testutilserve.go | 7 ++++--- util/waitmap/waitmap.go | 2 +- util/waitmap/waitmap_test.go | 2 +- 23 files changed, 81 insertions(+), 63 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a43f7f3c..4a0b2c82 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -56,6 +56,10 @@ linters-settings: desc: The io/ioutil package has been deprecated. forbidigo: forbid: + - '^context\.WithCancel(# use context\.WithCancelCause instead)?$' + - '^context\.WithDeadline(# use context\.WithDeadline instead)?$' + - '^context\.WithTimeout(# use context\.WithTimeoutCause instead)?$' + - '^ctx\.Err(# use context\.Cause instead)?$' - '^fmt\.Errorf(# use errors\.Errorf instead)?$' - '^platforms\.DefaultString(# use platforms\.Format(platforms\.DefaultSpec()) instead\.)?$' gosec: diff --git a/build/invoke.go b/build/invoke.go index 8e917ee0..5c84188c 100644 --- a/build/invoke.go +++ b/build/invoke.go @@ -16,7 +16,7 @@ import ( type Container struct { cancelOnce sync.Once - containerCancel func() + containerCancel func(error) isUnavailable atomic.Bool initStarted atomic.Bool container gateway.Container @@ -31,18 +31,18 @@ func NewContainer(ctx context.Context, resultCtx *ResultHandle, cfg *controllera errCh := make(chan error) go func() { err := resultCtx.build(func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { - ctx, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancelCause(ctx) go func() { <-mainCtx.Done() - cancel() + cancel(errors.WithStack(context.Canceled)) }() containerCfg, err := resultCtx.getContainerConfig(cfg) if err != nil { return nil, err } - containerCtx, containerCancel := context.WithCancel(ctx) - defer containerCancel() + containerCtx, containerCancel := context.WithCancelCause(ctx) + defer containerCancel(errors.WithStack(context.Canceled)) bkContainer, err := c.NewContainer(containerCtx, containerCfg) if err != nil { return nil, err @@ -83,7 +83,7 @@ func (c *Container) Cancel() { c.markUnavailable() c.cancelOnce.Do(func() { if c.containerCancel != nil { - c.containerCancel() + c.containerCancel(errors.WithStack(context.Canceled)) } close(c.releaseCh) }) diff --git a/build/result.go b/build/result.go index c96ac6cf..cb38fdcf 100644 --- a/build/result.go +++ b/build/result.go @@ -82,7 +82,7 @@ func NewResultHandle(ctx context.Context, cc *client.Client, opt client.SolveOpt var respHandle *ResultHandle go func() { - defer cancel(context.Canceled) // ensure no dangling processes + defer func() { cancel(errors.WithStack(context.Canceled)) }() // ensure no dangling processes var res *gateway.Result var err error @@ -181,7 +181,7 @@ func NewResultHandle(ctx context.Context, cc *client.Client, opt client.SolveOpt case <-respHandle.done: case <-ctx.Done(): } - return nil, ctx.Err() + return nil, context.Cause(ctx) }, nil) if respHandle != nil { return diff --git a/builder/builder.go b/builder/builder.go index fb54886d..33d2bbef 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -522,8 +522,9 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre return nil, err } - timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() + cancelCtx, cancel := context.WithCancelCause(ctx) + timeoutCtx, _ := context.WithTimeoutCause(cancelCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() nodes, err := b.LoadNodes(timeoutCtx, WithData()) if err != nil { diff --git a/commands/bake.go b/commands/bake.go index 95316c14..4737c138 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -108,8 +108,8 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba return err } - ctx2, cancel := context.WithCancel(context.TODO()) - defer cancel() + ctx2, cancel := context.WithCancelCause(context.TODO()) + defer cancel(errors.WithStack(context.Canceled)) var nodes []builder.Node var progressConsoleDesc, progressTextDesc string diff --git a/commands/build.go b/commands/build.go index e6fdb2f5..7850c1b4 100644 --- a/commands/build.go +++ b/commands/build.go @@ -325,8 +325,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) } attributes := buildMetricAttributes(dockerCli, driverType, &options) - ctx2, cancel := context.WithCancel(context.TODO()) - defer cancel() + ctx2, cancel := context.WithCancelCause(context.TODO()) + defer func() { cancel(errors.WithStack(context.Canceled)) }() progressMode, err := options.toDisplayMode() if err != nil { return err diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index 81471810..53444382 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -173,8 +173,8 @@ func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, arg // new resolver cause need new auth r = imagetools.New(imageopt) - ctx2, cancel := context.WithCancel(context.TODO()) - defer cancel() + ctx2, cancel := context.WithCancelCause(context.TODO()) + defer func() { cancel(errors.WithStack(context.Canceled)) }() printer, err := progress.NewPrinter(ctx2, os.Stderr, progressui.DisplayMode(in.progress)) if err != nil { return err diff --git a/commands/inspect.go b/commands/inspect.go index 7215a34b..8b16195a 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -17,6 +17,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/debug" "github.com/docker/go-units" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -34,8 +35,9 @@ func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) e return err } - timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() + timeoutCtx, cancel := context.WithCancelCause(ctx) + timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() nodes, err := b.LoadNodes(timeoutCtx, builder.WithData()) if in.bootstrap { diff --git a/commands/ls.go b/commands/ls.go index f5295ac0..fa6ad54e 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -18,6 +18,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/formatter" + "github.com/pkg/errors" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -57,8 +58,9 @@ func runLs(ctx context.Context, dockerCli command.Cli, in lsOptions) error { return err } - timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() + timeoutCtx, cancel := context.WithCancelCause(ctx) + timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() eg, _ := errgroup.WithContext(timeoutCtx) for _, b := range builders { diff --git a/commands/rm.go b/commands/rm.go index 6987cc42..b7242804 100644 --- a/commands/rm.go +++ b/commands/rm.go @@ -150,8 +150,9 @@ func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, i return err } - timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() + timeoutCtx, cancel := context.WithCancelCause(ctx) + timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() eg, _ := errgroup.WithContext(timeoutCtx) for _, b := range builders { diff --git a/controller/local/controller.go b/controller/local/controller.go index 9d58b31f..31e0779c 100644 --- a/controller/local/controller.go +++ b/controller/local/controller.go @@ -109,7 +109,7 @@ func (b *localController) Invoke(ctx context.Context, sessionID string, pid stri // Attach containerIn to this process ioCancelledCh := make(chan struct{}) - proc.ForwardIO(&ioset.In{Stdin: ioIn, Stdout: ioOut, Stderr: ioErr}, func() { close(ioCancelledCh) }) + proc.ForwardIO(&ioset.In{Stdin: ioIn, Stdout: ioOut, Stderr: ioErr}, func(error) { close(ioCancelledCh) }) select { case <-ioCancelledCh: @@ -117,7 +117,7 @@ func (b *localController) Invoke(ctx context.Context, sessionID string, pid stri case err := <-proc.Done(): return err case <-ctx.Done(): - return ctx.Err() + return context.Cause(ctx) } } diff --git a/controller/processes/processes.go b/controller/processes/processes.go index 3cd9e14f..e5acc708 100644 --- a/controller/processes/processes.go +++ b/controller/processes/processes.go @@ -18,16 +18,16 @@ type Process struct { invokeConfig *pb.InvokeConfig errCh chan error processCancel func() - serveIOCancel func() + serveIOCancel func(error) } // ForwardIO forwards process's io to the specified reader/writer. // Optionally specify ioCancelCallback which will be called when // the process closes the specified IO. This will be useful for additional cleanup. -func (p *Process) ForwardIO(in *ioset.In, ioCancelCallback func()) { +func (p *Process) ForwardIO(in *ioset.In, ioCancelCallback func(error)) { p.inEnd.SetIn(in) if f := p.serveIOCancel; f != nil { - f() + f(errors.WithStack(context.Canceled)) } p.serveIOCancel = ioCancelCallback } @@ -124,9 +124,16 @@ func (m *Manager) StartProcess(pid string, resultCtx *build.ResultHandle, cfg *p f.SetOut(&out) // Register process - ctx, cancel := context.WithCancel(context.TODO()) + ctx, cancel := context.WithCancelCause(context.TODO()) var cancelOnce sync.Once - processCancelFunc := func() { cancelOnce.Do(func() { cancel(); f.Close(); in.Close(); out.Close() }) } + processCancelFunc := func() { + cancelOnce.Do(func() { + cancel(errors.WithStack(context.Canceled)) + f.Close() + in.Close() + out.Close() + }) + } p := &Process{ inEnd: f, invokeConfig: cfg, diff --git a/controller/remote/controller.go b/controller/remote/controller.go index 224805e1..fd50d9d3 100644 --- a/controller/remote/controller.go +++ b/controller/remote/controller.go @@ -62,9 +62,10 @@ func NewRemoteBuildxController(ctx context.Context, dockerCli command.Cli, opts serverRoot := filepath.Join(rootDir, "shared") // connect to buildx server if it is already running - ctx2, cancel := context.WithTimeout(ctx, 1*time.Second) + ctx2, cancel := context.WithCancelCause(ctx) + ctx2, _ = context.WithTimeoutCause(ctx2, 1*time.Second, errors.WithStack(context.DeadlineExceeded)) c, err := newBuildxClientAndCheck(ctx2, filepath.Join(serverRoot, defaultSocketFilename)) - cancel() + cancel(errors.WithStack(context.Canceled)) if err != nil { if !errors.Is(err, context.DeadlineExceeded) { return nil, errors.Wrap(err, "cannot connect to the buildx server") @@ -90,9 +91,10 @@ func NewRemoteBuildxController(ctx context.Context, dockerCli command.Cli, opts go wait() // wait for buildx server to be ready - ctx2, cancel = context.WithTimeout(ctx, 10*time.Second) + ctx2, cancel = context.WithCancelCause(ctx) + ctx2, _ = context.WithTimeoutCause(ctx2, 10*time.Second, errors.WithStack(context.DeadlineExceeded)) c, err = newBuildxClientAndCheck(ctx2, filepath.Join(serverRoot, defaultSocketFilename)) - cancel() + cancel(errors.WithStack(context.Canceled)) if err != nil { return errors.Wrap(err, "cannot connect to the buildx server") } diff --git a/controller/remote/io.go b/controller/remote/io.go index 9a29534f..8685a772 100644 --- a/controller/remote/io.go +++ b/controller/remote/io.go @@ -344,7 +344,7 @@ func receive(ctx context.Context, stream msgStream) (*pb.Message, error) { case err := <-errCh: return nil, err case <-ctx.Done(): - return nil, ctx.Err() + return nil, context.Cause(ctx) } } diff --git a/controller/remote/server.go b/controller/remote/server.go index a53486ad..e18e43b5 100644 --- a/controller/remote/server.go +++ b/controller/remote/server.go @@ -37,7 +37,7 @@ type Server struct { type session struct { buildOnGoing atomic.Bool statusChan chan *pb.StatusResponse - cancelBuild func() + cancelBuild func(error) buildOptions *pb.BuildOptions inputPipe *io.PipeWriter @@ -109,7 +109,7 @@ func (m *Server) Disconnect(ctx context.Context, req *pb.DisconnectRequest) (res m.sessionMu.Lock() if s, ok := m.session[sessionID]; ok { if s.cancelBuild != nil { - s.cancelBuild() + s.cancelBuild(errors.WithStack(context.Canceled)) } s.cancelRunningProcesses() if s.result != nil { @@ -127,7 +127,7 @@ func (m *Server) Close() error { for k := range m.session { if s, ok := m.session[k]; ok { if s.cancelBuild != nil { - s.cancelBuild() + s.cancelBuild(errors.WithStack(context.Canceled)) } s.cancelRunningProcesses() } @@ -199,8 +199,8 @@ func (m *Server) Build(ctx context.Context, req *pb.BuildRequest) (*pb.BuildResp pw := pb.NewProgressWriter(statusChan) // Build the specified request - ctx, cancel := context.WithCancel(ctx) - defer cancel() + ctx, cancel := context.WithCancelCause(ctx) + defer func() { cancel(errors.WithStack(context.Canceled)) }() resp, res, _, buildErr := m.buildFunc(ctx, req.Options, inR, pw) m.sessionMu.Lock() if s, ok := m.session[sessionID]; ok { @@ -341,7 +341,7 @@ func (m *Server) Input(stream pb.Controller_InputServer) (err error) { select { case msg = <-msgCh: case <-ctx.Done(): - return errors.Wrap(ctx.Err(), "canceled") + return context.Cause(ctx) } if msg == nil { return nil @@ -370,9 +370,9 @@ func (m *Server) Invoke(srv pb.Controller_InvokeServer) error { initDoneCh := make(chan *processes.Process) initErrCh := make(chan error) eg, egCtx := errgroup.WithContext(context.TODO()) - srvIOCtx, srvIOCancel := context.WithCancel(egCtx) + srvIOCtx, srvIOCancel := context.WithCancelCause(egCtx) eg.Go(func() error { - defer srvIOCancel() + defer srvIOCancel(errors.WithStack(context.Canceled)) return serveIO(srvIOCtx, srv, func(initMessage *pb.InitMessage) (retErr error) { defer func() { if retErr != nil { @@ -418,7 +418,7 @@ func (m *Server) Invoke(srv pb.Controller_InvokeServer) error { }) }) eg.Go(func() (rErr error) { - defer srvIOCancel() + defer srvIOCancel(errors.WithStack(context.Canceled)) // Wait for init done var proc *processes.Process select { diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index b0ac0720..9705ce5d 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -212,7 +212,7 @@ func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error { } select { case <-ctx.Done(): - return ctx.Err() + return context.Cause(ctx) case <-time.After(time.Duration(try*120) * time.Millisecond): try++ continue diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index 289f17cd..6629cfcc 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -112,7 +112,7 @@ func (d *Driver) wait(ctx context.Context) error { for { select { case <-ctx.Done(): - return ctx.Err() + return context.Cause(ctx) case <-timeoutChan: return err case <-ticker.C: diff --git a/driver/remote/driver.go b/driver/remote/driver.go index eaf47e85..5fa9c70f 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -47,8 +47,9 @@ func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error { return err } return progress.Wrap("[internal] waiting for connection", l, func(_ progress.SubLogger) error { - ctx, cancel := context.WithTimeout(ctx, 20*time.Second) - defer cancel() + cancelCtx, cancel := context.WithCancelCause(ctx) + ctx, _ := context.WithTimeoutCause(cancelCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() return c.Wait(ctx) }) } diff --git a/monitor/monitor.go b/monitor/monitor.go index 6279eeb1..afc4af39 100644 --- a/monitor/monitor.go +++ b/monitor/monitor.go @@ -326,7 +326,7 @@ func (m *monitor) invoke(ctx context.Context, pid string, cfg *controllerapi.Inv if m.AttachedSessionID() == "" { return nil } - invokeCtx, invokeCancel := context.WithCancel(ctx) + invokeCtx, invokeCancel := context.WithCancelCause(ctx) containerIn, containerOut := ioset.Pipe() m.invokeIO.SetOut(&containerOut) @@ -336,7 +336,7 @@ func (m *monitor) invoke(ctx context.Context, pid string, cfg *controllerapi.Inv cancelOnce.Do(func() { containerIn.Close() m.invokeIO.SetOut(nil) - invokeCancel() + invokeCancel(errors.WithStack(context.Canceled)) }) <-waitInvokeDoneCh } diff --git a/util/dockerutil/client.go b/util/dockerutil/client.go index 831545b0..ff3f7d7e 100644 --- a/util/dockerutil/client.go +++ b/util/dockerutil/client.go @@ -41,7 +41,6 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri pr, pw := io.Pipe() done := make(chan struct{}) - ctx, cancel := context.WithCancel(ctx) var w *waitingWriter w = &waitingWriter{ PipeWriter: pw, @@ -67,8 +66,7 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri handleErr(err) } }, - done: done, - cancel: cancel, + done: done, } return w, func() { pr.Close() @@ -101,12 +99,11 @@ func (c *Client) features(ctx context.Context, name string) map[Feature]bool { type waitingWriter struct { *io.PipeWriter - f func() - once sync.Once - mu sync.Mutex - err error - done chan struct{} - cancel func() + f func() + once sync.Once + mu sync.Mutex + err error + done chan struct{} } func (w *waitingWriter) Write(dt []byte) (int, error) { diff --git a/util/gitutil/testutilserve.go b/util/gitutil/testutilserve.go index 4f8d42dd..9f876d05 100644 --- a/util/gitutil/testutilserve.go +++ b/util/gitutil/testutilserve.go @@ -7,6 +7,7 @@ import ( "net/http" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -25,7 +26,7 @@ func WithAccessToken(token string) GitServeOpt { func GitServeHTTP(c *Git, t testing.TB, opts ...GitServeOpt) (url string) { t.Helper() gitUpdateServerInfo(c, t) - ctx, cancel := context.WithCancel(context.TODO()) + ctx, cancel := context.WithCancelCause(context.TODO()) gs := &gitServe{} for _, opt := range opts { @@ -38,7 +39,7 @@ func GitServeHTTP(c *Git, t testing.TB, opts ...GitServeOpt) (url string) { name := "test.git" dir, err := c.GitDir() if err != nil { - cancel() + cancel(err) } var addr string @@ -84,7 +85,7 @@ func GitServeHTTP(c *Git, t testing.TB, opts ...GitServeOpt) (url string) { <-ready t.Cleanup(func() { - cancel() + cancel(errors.Errorf("cleanup")) <-done }) return fmt.Sprintf("http://%s/%s", addr, name) diff --git a/util/waitmap/waitmap.go b/util/waitmap/waitmap.go index c34b8f0c..582c8218 100644 --- a/util/waitmap/waitmap.go +++ b/util/waitmap/waitmap.go @@ -61,7 +61,7 @@ func (m *Map) Get(ctx context.Context, keys ...string) (map[string]interface{}, m.mu.Unlock() select { case <-ctx.Done(): - return nil, ctx.Err() + return nil, context.Cause(ctx) case <-ch: m.mu.Lock() } diff --git a/util/waitmap/waitmap_test.go b/util/waitmap/waitmap_test.go index 319be611..50f2f392 100644 --- a/util/waitmap/waitmap_test.go +++ b/util/waitmap/waitmap_test.go @@ -34,7 +34,7 @@ func TestTimeout(t *testing.T) { m.Set("foo", "bar") - ctx, cancel := context.WithTimeout(context.TODO(), 100*time.Millisecond) + ctx, cancel := context.WithTimeoutCause(context.TODO(), 100*time.Millisecond, nil) defer cancel() _, err := m.Get(ctx, "bar") From 58fd190c31eab0c9cc5a347f654718d1f191925a Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 18:29:04 -0800 Subject: [PATCH 5/5] lint: enable importas rules from buildkit Signed-off-by: Tonis Tiigi --- .golangci.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 4a0b2c82..b31d72fa 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -34,6 +34,16 @@ linters-settings: - "appendAssign" - "singleCaseSwitch" - "exitAfterDefer" # FIXME + importas: + alias: + # Enforce alias to prevent it accidentally being used instead of + # buildkit errdefs package (or vice-versa). + - pkg: "github.com/containerd/errdefs" + alias: "cerrdefs" + - pkg: "github.com/opencontainers/image-spec/specs-go/v1" + alias: "ocispecs" + - pkg: "github.com/opencontainers/go-digest" + alias: "digest" govet: enable: - nilness