From 98e1113c6deb4ac92eb58c2816aca3738ab42fff Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 13 May 2026 10:48:05 -0700 Subject: [PATCH] lint: modernize Go lint findings Update non-generated code for the newer lint recommendations by using typed atomic values, strings.Cut, and slices.Backward where applicable. Signed-off-by: Tonis Tiigi --- client/build_test.go | 2 +- control/control.go | 9 ++++---- executor/containerdexecutor/executor_unix.go | 9 ++++---- exporter/tar/export.go | 5 +++-- frontend/gateway/container/container.go | 8 +++---- session/grpc.go | 4 ++-- solver/llbsolver/ops/exec.go | 4 ++-- solver/result.go | 20 +++++++++--------- source/git/identifier.go | 6 +++--- source/git/source.go | 5 +++-- util/contentutil/copy.go | 4 ++-- util/network/cniprovider/cni_linux.go | 4 ++-- util/push/push.go | 5 +++-- util/resolvconf/resolvconf.go | 4 ++-- util/resolver/authorizer.go | 3 ++- util/resolver/pool.go | 5 ++--- util/throttle/throttle_test.go | 22 ++++++++++---------- 17 files changed, 61 insertions(+), 58 deletions(-) diff --git a/client/build_test.go b/client/build_test.go index 95238e376..461d6cbcb 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -2349,7 +2349,7 @@ func testClientGatewayCanceledCredentialsCallbackReturns(t *testing.T, sb integr t.Cleanup(proxyServer.Close) ref := strings.TrimPrefix(proxyServer.URL, "http://") + "/" + repo + ":latest" - host := strings.SplitN(ref, "/", 2)[0] + host, _, _ := strings.Cut(ref, "/") started := make(chan struct{}) release := make(chan struct{}) diff --git a/control/control.go b/control/control.go index caf6f045d..e4704fb39 100644 --- a/control/control.go +++ b/control/control.go @@ -81,8 +81,7 @@ type Opt struct { } type Controller struct { // TODO: ControlService - // buildCount needs to be 64bit aligned - buildCount int64 + buildCount atomic.Int64 opt Opt solver *llbsolver.Solver history *history.Queue @@ -213,7 +212,7 @@ func (c *Controller) releaseUnreferencedCache(ctx context.Context) error { } func (c *Controller) Prune(req *controlapi.PruneRequest, stream controlapi.Control_PruneServer) error { - if atomic.LoadInt64(&c.buildCount) == 0 { + if c.buildCount.Load() == 0 { imageutil.CancelCacheLeases() } @@ -383,8 +382,8 @@ func translateLegacySolveRequest(req *controlapi.SolveRequest) { func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*controlapi.SolveResponse, error) { defer trace.StartRegion(ctx, "Solve").End() trace.Logf(ctx, "Request", "solve request: %v", req.Ref) - atomic.AddInt64(&c.buildCount, 1) - defer atomic.AddInt64(&c.buildCount, -1) + c.buildCount.Add(1) + defer c.buildCount.Add(-1) if req.Cache == nil { req.Cache = &controlapi.CacheOptions{} // make sure cache options are initialized diff --git a/executor/containerdexecutor/executor_unix.go b/executor/containerdexecutor/executor_unix.go index ef0e5e630..328f0568c 100644 --- a/executor/containerdexecutor/executor_unix.go +++ b/executor/containerdexecutor/executor_unix.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "slices" ctd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/mount" @@ -46,8 +47,8 @@ func getUserSpec(user, rootfsPath string) (specs.User, error) { func (w *containerdExecutor) prepareExecutionEnv(ctx context.Context, rootMount executor.Mount, mounts []executor.Mount, meta executor.Meta, details *containerState, netMode pb.NetMode) (string, string, func(), error) { var releasers []func() releaseAll := func() { - for i := len(releasers) - 1; i >= 0; i-- { - releasers[i]() + for _, release := range slices.Backward(releasers) { + release() } } @@ -133,8 +134,8 @@ func (w *containerdExecutor) ensureCWD(details *containerState, meta executor.Me func (w *containerdExecutor) createOCISpec(ctx context.Context, id, resolvConf, hostsFile string, namespace network.Namespace, mounts []executor.Mount, meta executor.Meta, details *containerState) (*specs.Spec, func(), error) { var releasers []func() releaseAll := func() { - for i := len(releasers) - 1; i >= 0; i-- { - releasers[i]() + for _, release := range slices.Backward(releasers) { + release() } } diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 02000593b..c3bc64dca 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -3,6 +3,7 @@ package local import ( "context" "os" + "slices" "strings" "time" @@ -81,8 +82,8 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source var defers []func() error defer func() { - for i := len(defers) - 1; i >= 0; i-- { - defers[i]() + for _, f := range slices.Backward(defers) { + f() } }() diff --git a/frontend/gateway/container/container.go b/frontend/gateway/container/container.go index 042b7e793..458e3d445 100644 --- a/frontend/gateway/container/container.go +++ b/frontend/gateway/container/container.go @@ -97,8 +97,8 @@ func NewContainer(ctx context.Context, cm cache.Manager, exec executor.Executor, return cm.New(ctx, ref, g) }, platform.OS) if err != nil { - for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order - p.Actives[i].Ref.Release(context.TODO()) + for _, active := range slices.Backward(p.Actives) { // call in LIFO order + active.Ref.Release(context.TODO()) } for _, o := range p.OutputRefs { o.Ref.Release(context.TODO()) @@ -435,8 +435,8 @@ func (gwCtr *gatewayContainer) Release(ctx context.Context) error { err1 := gwCtr.errGroup.Wait() var err2 error - for i := len(gwCtr.cleanup) - 1; i >= 0; i-- { // call in LIFO order - err := gwCtr.cleanup[i]() + for _, cleanup := range slices.Backward(gwCtr.cleanup) { // call in LIFO order + err := cleanup() if err2 == nil { err2 = err } diff --git a/session/grpc.go b/session/grpc.go index 8eaaa4f1d..5e3291f58 100644 --- a/session/grpc.go +++ b/session/grpc.go @@ -67,9 +67,9 @@ func serve(ctx context.Context, grpcServer *grpc.Server, conn net.Conn) { } func grpcClientConn(ctx context.Context, conn net.Conn, opts map[string][]string) (context.Context, *grpc.ClientConn, error) { - var dialCount int64 + var dialCount atomic.Int64 dialer := grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { - if c := atomic.AddInt64(&dialCount, 1); c > 1 { + if c := dialCount.Add(1); c > 1 { return nil, errors.Errorf("only one connection allowed") } return conn, nil diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 09b37702c..7a79ae076 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -412,8 +412,8 @@ func (e *ExecOp) Exec(ctx context.Context, jobCtx solver.JobContext, inputs []so err = errdefs.WithExecError(err, execInputs, execMounts) } else { // Only release actives if err is nil. - for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order - p.Actives[i].Ref.Release(context.TODO()) + for _, active := range slices.Backward(p.Actives) { // call in LIFO order + active.Ref.Release(context.TODO()) } } for _, o := range p.OutputRefs { diff --git a/solver/result.go b/solver/result.go index e5f4b57e1..5216972ea 100644 --- a/solver/result.go +++ b/solver/result.go @@ -36,23 +36,23 @@ func (r *SharedResult) Release(ctx context.Context) error { } func dup(res Result) (Result, Result) { - sem := int64(0) + var sem atomic.Int64 return &splitResult{Result: res, sem: &sem}, &splitResult{Result: res, sem: &sem} } type splitResult struct { - released int64 - sem *int64 + released atomic.Int64 + sem *atomic.Int64 Result } func (r *splitResult) Release(ctx context.Context) error { - if atomic.AddInt64(&r.released, 1) > 1 { + if r.released.Add(1) > 1 { err := errors.Errorf("releasing already released reference %+v", r.ID()) bklog.G(ctx).Error(err) return err } - if atomic.AddInt64(r.sem, 1) == 2 { + if r.sem.Add(1) == 2 { return r.Result.Release(ctx) } return nil @@ -110,24 +110,24 @@ type SharedCachedResult struct { } type splitResultProxy struct { - released int64 - sem *int64 + released atomic.Int64 + sem *atomic.Int64 ResultProxy } func (r *splitResultProxy) Release(ctx context.Context) error { - if atomic.AddInt64(&r.released, 1) > 1 { + if r.released.Add(1) > 1 { err := errors.New("releasing already released reference") bklog.G(ctx).Error(err) return err } - if atomic.AddInt64(r.sem, 1) == 2 { + if r.sem.Add(1) == 2 { return r.ResultProxy.Release(ctx) } return nil } func SplitResultProxy(res ResultProxy) (ResultProxy, ResultProxy) { - sem := int64(0) + var sem atomic.Int64 return &splitResultProxy{ResultProxy: res, sem: &sem}, &splitResultProxy{ResultProxy: res, sem: &sem} } diff --git a/source/git/identifier.go b/source/git/identifier.go index 2fc875b97..3563da351 100644 --- a/source/git/identifier.go +++ b/source/git/identifier.go @@ -146,11 +146,11 @@ func validateBundleAttrs(id *GitIdentifier) error { // a colon after '@' (the digest) and confuses url.Parse. func splitBundleLocator(raw string) (string, string) { const sep = "://" - i := strings.Index(raw, sep) - if i < 0 { + scheme, body, ok := strings.Cut(raw, sep) + if !ok { return "", "" } - return raw[:i], raw[i+len(sep):] + return scheme, body } // parseBundleLocator extracts the scheme, normalized reference string, the diff --git a/source/git/source.go b/source/git/source.go index 2f33bc5ab..81d15fc57 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -13,6 +13,7 @@ import ( "path" "path/filepath" "regexp" + "slices" "strconv" "strings" "time" @@ -1420,8 +1421,8 @@ func resetSnapshotMtimes(root *os.Root, t time.Time) error { if err != nil { return err } - for i := len(dirs) - 1; i >= 0; i-- { - if err := root.Chtimes(dirs[i], t, t); err != nil { + for _, dir := range slices.Backward(dirs) { + if err := root.Chtimes(dir, t, t); err != nil { return err } } diff --git a/util/contentutil/copy.go b/util/contentutil/copy.go index 1d575d4b5..5ec7b21bc 100644 --- a/util/contentutil/copy.go +++ b/util/contentutil/copy.go @@ -3,6 +3,7 @@ package contentutil import ( "context" "io" + "slices" "strings" "sync" @@ -117,8 +118,7 @@ func copyChain(ctx context.Context, ingester content.Ingester, provider content. return errors.WithStack(err) } - for i := len(manifestStack) - 1; i >= 0; i-- { - desc := manifestStack[i] + for _, desc := range slices.Backward(manifestStack) { if err := Copy(ctx, ingester, provider, desc, "", nil); err != nil { return errors.WithStack(err) } diff --git a/util/network/cniprovider/cni_linux.go b/util/network/cniprovider/cni_linux.go index 3279a9017..24f2f66b4 100644 --- a/util/network/cniprovider/cni_linux.go +++ b/util/network/cniprovider/cni_linux.go @@ -88,7 +88,7 @@ func withDetachedNetNSIfAny(ctx context.Context, fn func(context.Context) error) return err } defer root.Close() - if _, err := root.Lstat("netns"); !errors.Is(err, os.ErrNotExist) { + if _, err := root.Lstat("netns"); err == nil { detachedNetNS := filepath.Join(stateDir, "netns") return ns.WithNetNSPath(detachedNetNS, func(_ ns.NetNS) error { ctx := context.WithValue(ctx, contextKeyDetachedNetNS, detachedNetNS) @@ -97,7 +97,7 @@ func withDetachedNetNSIfAny(ctx context.Context, fn func(context.Context) error) bklog.G(ctx).WithError(err2).Debugf("Leaving RootlessKit's detached netns %q", detachedNetNS) return err2 }) - } else if err != nil { + } else if !errors.Is(err, os.ErrNotExist) { return err } } diff --git a/util/push/push.go b/util/push/push.go index 77788da20..0ffe86757 100644 --- a/util/push/push.go +++ b/util/push/push.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" "sync" @@ -138,8 +139,8 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content } mfstDone := progress.OneOff(ctx, fmt.Sprintf("pushing manifest for %s", ref)) - for i := len(manifestStack) - 1; i >= 0; i-- { - if _, err := pushHandler(ctx, manifestStack[i]); err != nil { + for _, desc := range slices.Backward(manifestStack) { + if _, err := pushHandler(ctx, desc); err != nil { return mfstDone(err) } } diff --git a/util/resolvconf/resolvconf.go b/util/resolvconf/resolvconf.go index c00a50c7d..43cd936be 100644 --- a/util/resolvconf/resolvconf.go +++ b/util/resolvconf/resolvconf.go @@ -174,8 +174,8 @@ func (rc *ResolvConf) Options() []string { // Option("ndots") -> ("1", true) // Option("edns0") -> ("", true) func (rc *ResolvConf) Option(search string) (string, bool) { - for i := len(rc.options) - 1; i >= 0; i-- { - k, v, _ := strings.Cut(rc.options[i], ":") + for _, option := range slices.Backward(rc.options) { + k, v, _ := strings.Cut(option, ":") if k == search { return v, true } diff --git a/util/resolver/authorizer.go b/util/resolver/authorizer.go index 4a62e5b01..da772460b 100644 --- a/util/resolver/authorizer.go +++ b/util/resolver/authorizer.go @@ -9,6 +9,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "time" "github.com/containerd/containerd/v2/core/remotes/docker" @@ -28,7 +29,7 @@ import ( const defaultExpiration = 60 type authHandlerNS struct { - counter int64 // needs to be 64bit aligned for 32bit systems + counter atomic.Int64 fetchers map[string]*authFetcher muHandlers sync.Mutex diff --git a/util/resolver/pool.go b/util/resolver/pool.go index fdc0fb3a0..06d8be655 100644 --- a/util/resolver/pool.go +++ b/util/resolver/pool.go @@ -6,7 +6,6 @@ import ( "net/http" "strings" "sync" - "sync/atomic" "time" "github.com/containerd/containerd/v2/core/images" @@ -233,7 +232,7 @@ func (r *Resolver) WithImageStore(is images.Store, mode ResolveMode) *Resolver { // Fetcher returns a new fetcher for the provided reference. func (r *Resolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) { - if atomic.LoadInt64(&r.handler.counter) == 0 { + if r.handler.counter.Load() == 0 { r.Resolve(ctx, ref) } return r.Resolver.Fetcher(ctx, ref) @@ -249,7 +248,7 @@ func (r *Resolver) Resolve(ctx context.Context, ref string) (string, ocispecs.De n, desc, err := r.Resolver.Resolve(ctx, ref) if err == nil { - atomic.AddInt64(&r.handler.counter, 1) + r.handler.counter.Add(1) return n, desc, nil } diff --git a/util/throttle/throttle_test.go b/util/throttle/throttle_test.go index fd9d013ae..9d06729e0 100644 --- a/util/throttle/throttle_test.go +++ b/util/throttle/throttle_test.go @@ -11,9 +11,9 @@ import ( func TestThrottle(t *testing.T) { t.Parallel() - var i int64 + var i atomic.Int64 f := func() { - atomic.AddInt64(&i, 1) + i.Add(1) } f = Throttle(50*time.Millisecond, f) @@ -21,14 +21,14 @@ func TestThrottle(t *testing.T) { f() f() - require.Equal(t, int64(0), atomic.LoadInt64(&i)) + require.Equal(t, int64(0), i.Load()) // test that i is never incremented twice and at least once in next 600ms retries := 0 for { require.Less(t, retries, 10) time.Sleep(60 * time.Millisecond) - v := atomic.LoadInt64(&i) + v := i.Load() require.LessOrEqual(t, v, int64(1)) if v == 1 { break @@ -36,7 +36,7 @@ func TestThrottle(t *testing.T) { retries++ } - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) f() @@ -44,7 +44,7 @@ func TestThrottle(t *testing.T) { for { require.Less(t, retries, 10) time.Sleep(60 * time.Millisecond) - v := atomic.LoadInt64(&i) + v := i.Load() if v == 2 { break } @@ -55,9 +55,9 @@ func TestThrottle(t *testing.T) { func TestAfter(t *testing.T) { t.Parallel() - var i int64 + var i atomic.Int64 f := func() { - atomic.AddInt64(&i, 1) + i.Add(1) } f = After(100*time.Millisecond, f) @@ -65,11 +65,11 @@ func TestAfter(t *testing.T) { f() time.Sleep(10 * time.Millisecond) - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) f() time.Sleep(10 * time.Millisecond) - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) time.Sleep(200 * time.Millisecond) - require.Equal(t, int64(2), atomic.LoadInt64(&i)) + require.Equal(t, int64(2), i.Load()) }