update golangci-lint to v1.50.1 (for go 1.18 compatibility)

.golangci.yml

@@ -1,14 +1,15 @@
 run:
   tests: false
+  timeout: 10m
 linters: 
   disable-all: true
   enable:
     - misspell
     - gofmt
     - goimports
-    - golint
     - ineffassign
-    - deadcode
+    - revive
     - unconvert
+    - unused
     - govet
-      
+

agent/csi/plugin/manager.go

@@ -18,9 +18,9 @@ const (
 	DockerCSIPluginCap = "csinode"
 )
 
-// PluginManager manages the multiple CSI plugins that may be in use on the
-// node. PluginManager should be thread-safe.
-type PluginManager interface {
+// Manager manages the multiple CSI plugins that may be in use on the
+// node. Manager should be thread-safe.
+type Manager interface {
 	// Get gets the plugin with the given name
 	Get(name string) (NodePlugin, error)
 
@@ -43,7 +43,7 @@ type pluginManager struct {
 	pg plugingetter.PluginGetter
 }
 
-func NewPluginManager(pg plugingetter.PluginGetter, secrets SecretGetter) PluginManager {
+func NewManager(pg plugingetter.PluginGetter, secrets SecretGetter) Manager {
 	return &pluginManager{
 		plugins:           map[string]NodePlugin{},
 		newNodePluginFunc: NewNodePlugin,

agent/csi/plugin/manager_deprecated.go

@@ -0,0 +1,11 @@
+package plugin
+
+// Deprecated: use [Manager].
+//
+//nolint:revive // exported: type name will be used as plugin.PluginManager by other packages
+type PluginManager = Manager
+
+// Deprecated: use [NewManager].
+//
+//nolint:unused
+var NewPluginManager = NewManager

agent/csi/plugin/manager_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/moby/swarmkit/v2/testutils"
 )
 
-var _ = Describe("PluginManager", func() {
+var _ = Describe("Manager", func() {
 	var (
 		pm *pluginManager
 		pg *testutils.FakePluginGetter

agent/csi/plugin/plugin_fake_test.go

@@ -39,7 +39,7 @@ func (f *fakeNodePlugin) NodeGetInfo(ctx context.Context) (*api.NodeCSIInfo, err
 }
 
 // these methods are all stubs, as they are not needed for testing the
-// PluginManager.
+// Manager.
 func (f *fakeNodePlugin) GetPublishedPath(volumeID string) string {
 	return ""
 }

agent/csi/volumes.go

@@ -17,7 +17,7 @@ import (
 	"github.com/moby/swarmkit/v2/volumequeue"
 )
 
-const CSI_CALL_TIMEOUT = 15 * time.Second
+const csiCallTimeout = 15 * time.Second
 
 // volumeState keeps track of the state of a volume on this node.
 type volumeState struct {
@@ -39,8 +39,8 @@ type volumes struct {
 	// volumes is a mapping of volume ID to volumeState
 	volumes map[string]volumeState
 
-	// plugins is the PluginManager, which provides translation to the CSI RPCs
-	plugins plugin.PluginManager
+	// plugins is the Manager, which provides translation to the CSI RPCs
+	plugins plugin.Manager
 
 	// pendingVolumes is a VolumeQueue which manages which volumes are
 	// processed and when.
@@ -51,7 +51,7 @@ type volumes struct {
 func NewManager(pg plugingetter.PluginGetter, secrets exec.SecretGetter) exec.VolumesManager {
 	r := &volumes{
 		volumes:        map[string]volumeState{},
-		plugins:        plugin.NewPluginManager(pg, secrets),
+		plugins:        plugin.NewManager(pg, secrets),
 		pendingVolumes: volumequeue.NewVolumeQueue(),
 	}
 	go r.retryVolumes()
@@ -107,7 +107,7 @@ func (r *volumes) tryVolume(ctx context.Context, id string, attempt uint) {
 	// These are too complicated to be worth the engineering effort at this
 	// time.
 
-	timeoutCtx, cancel := context.WithTimeout(ctx, CSI_CALL_TIMEOUT)
+	timeoutCtx, cancel := context.WithTimeout(ctx, csiCallTimeout)
 	// always gotta call the WithTimeout cancel
 	defer cancel()
 

agent/exec/controller_stub.go

@@ -21,7 +21,6 @@ type StubController struct {
 	RemoveFn    func(ctx context.Context) error
 	CloseFn     func() error
 	calls       map[string]int
-	cstatus     *api.ContainerStatus
 }
 
 // NewStubController returns an initialized StubController

agent/exec/dockerapi/container.go

@@ -76,6 +76,7 @@ func (c *containerConfig) setTask(n *api.NodeDescription, t *api.Task) error {
 	return nil
 }
 
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (c *containerConfig) endpoint() *api.Endpoint {
 	return c.task.Endpoint
 }
@@ -479,6 +480,7 @@ func (c *containerConfig) resources() enginecontainer.Resources {
 	return resources
 }
 
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (c *containerConfig) virtualIP(networkID string) string {
 	if c.task.Endpoint == nil {
 		return ""

agent/reporter.go

@@ -15,7 +15,7 @@ type StatusReporter interface {
 	UpdateTaskStatus(ctx context.Context, taskID string, status *api.TaskStatus) error
 }
 
-// Reporter recieves update to both task and volume status.
+// Reporter receives update to both task and volume status.
 type Reporter interface {
 	StatusReporter
 	ReportVolumeUnpublished(ctx context.Context, volumeID string) error
@@ -27,12 +27,15 @@ func (fn statusReporterFunc) UpdateTaskStatus(ctx context.Context, taskID string
 	return fn(ctx, taskID, status)
 }
 
+//nolint:unused // currently only used in tests.
 type volumeReporterFunc func(ctx context.Context, volumeID string) error
 
+//nolint:unused // currently only used in tests.
 func (fn volumeReporterFunc) ReportVolumeUnpublished(ctx context.Context, volumeID string) error {
 	return fn(ctx, volumeID)
 }
 
+//nolint:unused // currently only used in tests.
 type statusReporterCombined struct {
 	statusReporterFunc
 	volumeReporterFunc

agent/session.go

@@ -391,6 +391,7 @@ func (s *session) sendTaskStatus(ctx context.Context, taskID string, taskStatus
 	return nil
 }
 
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (s *session) sendTaskStatuses(ctx context.Context, updates ...*api.UpdateTaskStatusRequest_TaskStatusUpdate) ([]*api.UpdateTaskStatusRequest_TaskStatusUpdate, error) {
 	if len(updates) < 1 {
 		return nil, nil

agent/worker.go

@@ -57,7 +57,6 @@ type statusReporterKey struct {
 type worker struct {
 	db                *bolt.DB
 	executor          exec.Executor
-	publisher         exec.LogPublisher
 	listeners         map[*statusReporterKey]struct{}
 	taskevents        *watch.Queue
 	publisherProvider exec.LogPublisherProvider

ca/server.go

@@ -65,7 +65,6 @@ type Server struct {
 	signingMu sync.Mutex
 
 	// lets us monitor and finish root rotations
-	rootReconciler                  *rootRotationReconciler
 	rootReconciliationRetryInterval time.Duration
 }
 

direct.mk

@@ -37,8 +37,8 @@ version/version.go:
 .PHONY: setup
 setup: ## install dependencies
 	@echo "🐳 $@"
-	# install golangci-lint version 1.17.1 to ./bin/golangci-lint
-	@curl -fsSL https://raw.githubusercontent.com/golangci/golangci-lint/v1.17.1/install.sh | sh -s v1.17.1
+	# install golangci-lint version v1.50.1 to ./bin/golangci-lint
+	@curl -fsSL https://raw.githubusercontent.com/golangci/golangci-lint/v1.50.1/install.sh | sh -s v1.50.1
 	@(cd tools ; GO111MODULE=on go install github.com/containerd/protobuild)
 
 .PHONY: generate

integration/cluster.go → integration/cluster_test.go

@@ -9,9 +9,6 @@ import (
 	"sync"
 	"time"
 
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials"
-
 	"github.com/moby/swarmkit/v2/api"
 	"github.com/moby/swarmkit/v2/ca"
 	"github.com/moby/swarmkit/v2/identity"
@@ -20,6 +17,8 @@ import (
 	"github.com/moby/swarmkit/v2/node"
 	"github.com/moby/swarmkit/v2/testutils"
 	"github.com/sirupsen/logrus"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials"
 )
 
 const opsTimeout = 64 * time.Second

integration/integration_test.go

@@ -7,22 +7,20 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/moby/swarmkit/v2/node"
-
-	"reflect"
-
 	"github.com/cloudflare/cfssl/helpers"
 	events "github.com/docker/go-events"
 	"github.com/moby/swarmkit/v2/api"
 	"github.com/moby/swarmkit/v2/ca"
 	cautils "github.com/moby/swarmkit/v2/ca/testutils"
 	"github.com/moby/swarmkit/v2/identity"
 	"github.com/moby/swarmkit/v2/manager"
+	"github.com/moby/swarmkit/v2/node"
 	"github.com/moby/swarmkit/v2/testutils"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -399,7 +397,7 @@ func TestDemotePromoteLeader(t *testing.T) {
 	numManager--
 	pollClusterReady(t, cl, numWorker, numManager)
 
-	//promote former leader back
+	// promote former leader back
 	require.NoError(t, cl.SetNodeRole(leader.node.NodeID(), api.NodeRoleManager))
 	// agents 1, managers 3
 	numWorker--

integration/node.go → integration/node_test.go

@@ -8,13 +8,12 @@ import (
 	"runtime"
 	"strings"
 
-	"google.golang.org/grpc"
-
 	agentutils "github.com/moby/swarmkit/v2/agent/testutils"
 	"github.com/moby/swarmkit/v2/api"
 	"github.com/moby/swarmkit/v2/ca"
 	"github.com/moby/swarmkit/v2/node"
 	"github.com/moby/swarmkit/v2/testutils"
+	"google.golang.org/grpc"
 )
 
 // TestNode is representation of *agent.Node. It stores listeners, connections,

manager/allocator/cnmallocator/portallocator.go

@@ -297,10 +297,6 @@ func (pa *portAllocator) hostPublishPortsNeedUpdate(s *api.Service) bool {
 	return false
 }
 
-func (pa *portAllocator) isPortsAllocated(s *api.Service) bool {
-	return pa.isPortsAllocatedOnInit(s, false)
-}
-
 func (pa *portAllocator) isPortsAllocatedOnInit(s *api.Service, onInit bool) bool {
 	// If service has no user-defined endpoint and allocated endpoint,
 	// we assume it is allocated and return true.

manager/allocator/cnmallocator/portallocator_test.go

@@ -879,7 +879,7 @@ func TestIsPortsAllocated(t *testing.T) {
 
 	for _, singleTest := range testCases {
 		t.Run(singleTest.name, func(t *testing.T) {
-			expect := pa.isPortsAllocated(singleTest.input)
+			expect := pa.isPortsAllocatedOnInit(singleTest.input, false)
 			assert.Equal(t, expect, singleTest.expect)
 		})
 	}

manager/allocator/network.go

@@ -375,6 +375,7 @@ func isOverlayNetwork(n *api.Network) bool {
 	return false
 }
 
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (a *Allocator) getAllocatedNetworks() ([]*api.Network, error) {
 	var (
 		err               error
@@ -506,6 +507,7 @@ func (a *Allocator) allocateNodes(ctx context.Context, existingAddressesOnly boo
 	return nil
 }
 
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (a *Allocator) deallocateNodes(ctx context.Context) error {
 	var (
 		nodes []*api.Node

manager/scheduler/volumes.go

@@ -50,9 +50,11 @@ func newVolumeSet() *volumeSet {
 	}
 }
 
+// getVolume returns the volume object for the given ID as stored in the
+// volumeSet, or nil if none exists.
+//
+//nolint:unused // TODO(thaJeztah) this is currently unused: is it safe to remove?
 func (vs *volumeSet) getVolume(id string) *api.Volume {
-	// getVolume returns the volume object for the given ID as stored in the
-	// volumeSet, or nil if none exists
 	return vs.volumes[id].volume
 }
 
@@ -77,6 +79,7 @@ func (vs *volumeSet) addOrUpdateVolume(v *api.Volume) {
 	vs.byName[v.Spec.Annotations.Name] = v.ID
 }
 
+//nolint:unused // only used in tests.
 func (vs *volumeSet) removeVolume(volumeID string) {
 	if info, ok := vs.volumes[volumeID]; ok {
 		// if the volume exists in the set, look up its group ID and remove it

manager/state/raft/raft.go

@@ -132,8 +132,7 @@ type Node struct {
 	// RemovedFromRaft notifies about node deletion from raft cluster
 	RemovedFromRaft chan struct{}
 	cancelFunc      func()
-	// removeRaftCh notifies about node deletion from raft cluster
-	removeRaftCh        chan struct{}
+
 	removeRaftOnce      sync.Once
 	leadershipBroadcast *watch.Queue
 
@@ -1289,6 +1288,7 @@ func (n *Node) processRaftMessageLogger(ctx context.Context, msg *api.ProcessRaf
 	return log.G(ctx).WithFields(fields)
 }
 
+//nolint:unused // currently unused, but should be used again; see TODO in Node.ProcessRaftMessage
 func (n *Node) reportNewAddress(ctx context.Context, id uint64) error {
 	// too early
 	if !n.IsMember() {
@@ -1418,9 +1418,9 @@ func (n *Node) ProcessRaftMessage(ctx context.Context, msg *api.ProcessRaftMessa
 	// See https://github.com/docker/docker/issues/30455.
 	// This should be reenabled in the future with additional
 	// safeguards (perhaps storing multiple addresses per node).
-	//if err := n.reportNewAddress(ctx, msg.Message.From); err != nil {
+	// if err := n.reportNewAddress(ctx, msg.Message.From); err != nil {
 	//	log.G(ctx).WithError(err).Errorf("failed to report new address of %x to transport", msg.Message.From)
-	//}
+	// }
 
 	// Reject vote requests from unreachable peers
 	if msg.Message.Type == raftpb.MsgVote {