Update golangci and address complaints (#3500)

* feat(golangci): update to 1.53.3 * fix(siren): fix golangci complaints * fix(makefile): make run-server work * feat(golangci): enable gocritic * fix(s-m): make sure that qb.Query end up .Released --------- Co-authored-by: Dmitry Kropachev <[email protected]>
scylladb · Jul 26, 2023 · 389b408 · 389b408
1 parent 2076539
commit 389b408
Show file tree

Hide file tree

Showing 41 changed files with 154 additions and 137 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -28,6 +28,7 @@ linters-settings:
       - regexpSimplify
       - yodaStyleExpr
       - whyNoLint
+      - timeCmpSimplify
   lll:
     line-length: 180
   nestif:
@@ -36,6 +37,8 @@ linters-settings:
 linters:
   enable-all: true
   disable:
+    - depguard
+    - tagalign
     - deadcode
     - varcheck
     - structcheck
@@ -54,7 +57,6 @@ linters:
     - gas
     - gochecknoglobals
     - gochecknoinits
-    - gocritic
     - goerr113
     - golint
     - gomnd

diff --git a/Makefile b/Makefile
@@ -188,7 +188,7 @@ build-server: ## Build development server
 .PHONY: run-server
 run-server: build-server ## Build and run development server
 	@docker run --name "scylla_manager_server" \
-		--network scylla_manager_second --network scylla_manager_public \
+		--network scylla_manager_second \
 		-p "5080:5080" \
 		-p "5443:5443" \
 		-p "5090:5090" \
@@ -197,6 +197,7 @@ run-server: build-server ## Build and run development server
 		-v "$(PWD)/$(MANAGER_CONFIG):/etc/scylla-manager/scylla-manager.yaml:ro" \
 		-v "/tmp:/tmp" \
 		-i --read-only --rm scylladb/scylla-manager-dev scylla-manager
+	@docker network connect scylla_manager_public scylla-manager
 
 .PHONY: build
 build: build-cli build-agent build-server ## Build all project binaries

diff --git a/install-dependencies.sh b/install-dependencies.sh
@@ -35,7 +35,7 @@ rm -f "${LOCAL_BIN}"/*
 echo "==> Installing Go packages at ${LOCAL_BIN}"
 
 export GOBIN=${LOCAL_BIN}
-go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1
+go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
 go install github.com/goreleaser/[email protected]
 go install github.com/go-enry/go-license-detector/v4/cmd/license-detector@latest
 go install github.com/golang/mock/[email protected]

diff --git a/pkg/cmd/scylla-manager/root.go b/pkg/cmd/scylla-manager/root.go
@@ -41,20 +41,20 @@ var rootCmd = &cobra.Command{
 		// Print version and return
 		if rootArgs.version {
 			fmt.Fprintf(cmd.OutOrStdout(), "%s\n", pkg.Version())
-			return
+			return nil
 		}
 
 		// Read configuration
 		c, err := config.ParseConfigFiles(rootArgs.configFiles)
 		if err != nil {
 			runError = errors.Wrapf(err, "configuration %q", rootArgs.configFiles)
 			fmt.Fprintf(cmd.OutOrStderr(), "%s\n", runError)
-			return
+			return runError
 		}
 		if err := c.Validate(); err != nil {
 			runError = errors.Wrapf(err, "configuration %q", rootArgs.configFiles)
 			fmt.Fprintf(cmd.OutOrStderr(), "%s\n", runError)
-			return
+			return runError
 		}
 
 		// Create logger

diff --git a/pkg/managerclient/client.go b/pkg/managerclient/client.go
@@ -87,7 +87,7 @@ func NewClient(rawURL string, opts ...Option) (Client, error) {
 }
 
 // CreateCluster creates a new cluster.
-func (c Client) CreateCluster(ctx context.Context, cluster *Cluster) (string, error) {
+func (c *Client) CreateCluster(ctx context.Context, cluster *Cluster) (string, error) {
 	resp, err := c.operations.PostClusters(&operations.PostClustersParams{
 		Context: ctx,
 		Cluster: cluster,
@@ -105,7 +105,7 @@ func (c Client) CreateCluster(ctx context.Context, cluster *Cluster) (string, er
 }
 
 // GetCluster returns a cluster for a given ID.
-func (c Client) GetCluster(ctx context.Context, clusterID string) (*Cluster, error) {
+func (c *Client) GetCluster(ctx context.Context, clusterID string) (*Cluster, error) {
 	resp, err := c.operations.GetClusterClusterID(&operations.GetClusterClusterIDParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -118,7 +118,7 @@ func (c Client) GetCluster(ctx context.Context, clusterID string) (*Cluster, err
 }
 
 // UpdateCluster updates cluster.
-func (c Client) UpdateCluster(ctx context.Context, cluster *Cluster) error {
+func (c *Client) UpdateCluster(ctx context.Context, cluster *Cluster) error {
 	_, err := c.operations.PutClusterClusterID(&operations.PutClusterClusterIDParams{ // nolint: errcheck
 		Context:   ctx,
 		ClusterID: cluster.ID,
@@ -128,7 +128,7 @@ func (c Client) UpdateCluster(ctx context.Context, cluster *Cluster) error {
 }
 
 // DeleteCluster removes cluster.
-func (c Client) DeleteCluster(ctx context.Context, clusterID string) error {
+func (c *Client) DeleteCluster(ctx context.Context, clusterID string) error {
 	_, err := c.operations.DeleteClusterClusterID(&operations.DeleteClusterClusterIDParams{ // nolint: errcheck
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -137,7 +137,7 @@ func (c Client) DeleteCluster(ctx context.Context, clusterID string) error {
 }
 
 // DeleteClusterSecrets removes cluster secrets.
-func (c Client) DeleteClusterSecrets(ctx context.Context, clusterID string, cqlCreds, sslUserCert bool) error {
+func (c *Client) DeleteClusterSecrets(ctx context.Context, clusterID string, cqlCreds, sslUserCert bool) error {
 	ok := false
 	p := &operations.DeleteClusterClusterIDParams{
 		Context:   ctx,
@@ -161,7 +161,7 @@ func (c Client) DeleteClusterSecrets(ctx context.Context, clusterID string, cqlC
 }
 
 // ListClusters returns clusters.
-func (c Client) ListClusters(ctx context.Context) (ClusterSlice, error) {
+func (c *Client) ListClusters(ctx context.Context) (ClusterSlice, error) {
 	resp, err := c.operations.GetClusters(&operations.GetClustersParams{
 		Context: ctx,
 	})
@@ -173,7 +173,7 @@ func (c Client) ListClusters(ctx context.Context) (ClusterSlice, error) {
 }
 
 // ClusterStatus returns health check progress.
-func (c Client) ClusterStatus(ctx context.Context, clusterID string) (ClusterStatus, error) {
+func (c *Client) ClusterStatus(ctx context.Context, clusterID string) (ClusterStatus, error) {
 	resp, err := c.operations.GetClusterClusterIDStatus(&operations.GetClusterClusterIDStatusParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -338,7 +338,7 @@ func (c *Client) UpdateTask(ctx context.Context, clusterID string, t *Task) erro
 }
 
 // ListTasks returns tasks within a clusterID, optionally filtered by task type tp.
-func (c *Client) ListTasks(ctx context.Context, clusterID, taskType string, all bool, status string, taskID string) (TaskListItems, error) {
+func (c *Client) ListTasks(ctx context.Context, clusterID, taskType string, all bool, status, taskID string) (TaskListItems, error) {
 	resp, err := c.operations.GetClusterClusterIDTasks(&operations.GetClusterClusterIDTasksParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -456,7 +456,7 @@ func formatTaskList(tasks []*models.TaskListItem) string {
 }
 
 // RepairProgress returns repair progress.
-func (c Client) RepairProgress(ctx context.Context, clusterID, taskID, runID string) (RepairProgress, error) {
+func (c *Client) RepairProgress(ctx context.Context, clusterID, taskID, runID string) (RepairProgress, error) {
 	resp, err := c.operations.GetClusterClusterIDTaskRepairTaskIDRunID(&operations.GetClusterClusterIDTaskRepairTaskIDRunIDParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -473,7 +473,7 @@ func (c Client) RepairProgress(ctx context.Context, clusterID, taskID, runID str
 }
 
 // BackupProgress returns backup progress.
-func (c Client) BackupProgress(ctx context.Context, clusterID, taskID, runID string) (BackupProgress, error) {
+func (c *Client) BackupProgress(ctx context.Context, clusterID, taskID, runID string) (BackupProgress, error) {
 	tr := &models.TaskRunBackupProgress{
 		Progress: &models.BackupProgress{
 			Stage: "INIT",
@@ -508,7 +508,7 @@ func (c Client) BackupProgress(ctx context.Context, clusterID, taskID, runID str
 }
 
 // RestoreProgress returns restore progress.
-func (c Client) RestoreProgress(ctx context.Context, clusterID, taskID, runID string) (RestoreProgress, error) {
+func (c *Client) RestoreProgress(ctx context.Context, clusterID, taskID, runID string) (RestoreProgress, error) {
 	resp, err := c.operations.GetClusterClusterIDTaskRestoreTaskIDRunID(&operations.GetClusterClusterIDTaskRestoreTaskIDRunIDParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -525,7 +525,7 @@ func (c Client) RestoreProgress(ctx context.Context, clusterID, taskID, runID st
 }
 
 // ValidateBackupProgress returns validate backup progress.
-func (c Client) ValidateBackupProgress(ctx context.Context, clusterID, taskID, runID string) (ValidateBackupProgress, error) {
+func (c *Client) ValidateBackupProgress(ctx context.Context, clusterID, taskID, runID string) (ValidateBackupProgress, error) {
 	resp, err := c.operations.GetClusterClusterIDTaskValidateBackupTaskIDRunID(&operations.GetClusterClusterIDTaskValidateBackupTaskIDRunIDParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -542,7 +542,7 @@ func (c Client) ValidateBackupProgress(ctx context.Context, clusterID, taskID, r
 }
 
 // ListBackups returns listing of available backups.
-func (c Client) ListBackups(ctx context.Context, clusterID string,
+func (c *Client) ListBackups(ctx context.Context, clusterID string,
 	locations []string, allClusters bool, keyspace []string, minDate, maxDate time.Time,
 ) (BackupListItems, error) {
 	p := &operations.GetClusterClusterIDBackupsParams{
@@ -570,7 +570,7 @@ func (c Client) ListBackups(ctx context.Context, clusterID string,
 }
 
 // ListBackupFiles returns a listing of available backup files.
-func (c Client) ListBackupFiles(ctx context.Context, clusterID string,
+func (c *Client) ListBackupFiles(ctx context.Context, clusterID string,
 	locations []string, allClusters bool, keyspace []string, snapshotTag string,
 ) ([]*models.BackupFilesInfo, error) {
 	p := &operations.GetClusterClusterIDBackupsFilesParams{
@@ -593,7 +593,7 @@ func (c Client) ListBackupFiles(ctx context.Context, clusterID string,
 }
 
 // DeleteSnapshot deletes backup snapshot with all data associated with it.
-func (c Client) DeleteSnapshot(ctx context.Context, clusterID string,
+func (c *Client) DeleteSnapshot(ctx context.Context, clusterID string,
 	locations []string, snapshotTags []string,
 ) error {
 	p := &operations.DeleteClusterClusterIDBackupsParams{
@@ -608,7 +608,7 @@ func (c Client) DeleteSnapshot(ctx context.Context, clusterID string,
 }
 
 // Version returns server version.
-func (c Client) Version(ctx context.Context) (*models.Version, error) {
+func (c *Client) Version(ctx context.Context) (*models.Version, error) {
 	resp, err := c.operations.GetVersion(&operations.GetVersionParams{
 		Context: ctx,
 	})
@@ -620,7 +620,7 @@ func (c Client) Version(ctx context.Context) (*models.Version, error) {
 }
 
 // SetRepairIntensity updates ongoing repair intensity.
-func (c Client) SetRepairIntensity(ctx context.Context, clusterID string, intensity float64) error {
+func (c *Client) SetRepairIntensity(ctx context.Context, clusterID string, intensity float64) error {
 	p := &operations.PutClusterClusterIDRepairsIntensityParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -632,7 +632,7 @@ func (c Client) SetRepairIntensity(ctx context.Context, clusterID string, intens
 }
 
 // SetRepairParallel updates ongoing repair parallel disjoint host groups.
-func (c Client) SetRepairParallel(ctx context.Context, clusterID string, parallel int64) error {
+func (c *Client) SetRepairParallel(ctx context.Context, clusterID string, parallel int64) error {
 	p := &operations.PutClusterClusterIDRepairsParallelParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -644,7 +644,7 @@ func (c Client) SetRepairParallel(ctx context.Context, clusterID string, paralle
 }
 
 // IsSuspended returns true iff the current cluster is suspended.
-func (c Client) IsSuspended(ctx context.Context, clusterID string) (bool, error) {
+func (c *Client) IsSuspended(ctx context.Context, clusterID string) (bool, error) {
 	p := &operations.GetClusterClusterIDSuspendedParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -659,7 +659,7 @@ func (c Client) IsSuspended(ctx context.Context, clusterID string) (bool, error)
 }
 
 // Suspend updates cluster suspended property.
-func (c Client) Suspend(ctx context.Context, clusterID string) error {
+func (c *Client) Suspend(ctx context.Context, clusterID string) error {
 	p := &operations.PutClusterClusterIDSuspendedParams{
 		Context:   ctx,
 		ClusterID: clusterID,
@@ -671,7 +671,7 @@ func (c Client) Suspend(ctx context.Context, clusterID string) error {
 }
 
 // Resume updates cluster suspended property.
-func (c Client) Resume(ctx context.Context, clusterID string, startTasks bool) error {
+func (c *Client) Resume(ctx context.Context, clusterID string, startTasks bool) error {
 	p := &operations.PutClusterClusterIDSuspendedParams{
 		Context:    ctx,
 		ClusterID:  clusterID,

diff --git a/pkg/managerclient/utils.go b/pkg/managerclient/utils.go
@@ -126,7 +126,7 @@ func FormatTime(t strfmt.DateTime) string {
 	if isZero(t) {
 		return ""
 	}
-	return time.Time(t).Local().Format(rfc822WithSec)
+	return time.Time(t).Local().Format(rfc822WithSec) //nolint: gosmopolitan
 }
 
 // FormatTimePointer see FormatTime.

diff --git a/pkg/metrics/delete.go b/pkg/metrics/delete.go
@@ -16,15 +16,20 @@ type CollectorDeleter interface {
 // DeleteMatching removes metric instances with matching labels.
 func DeleteMatching(c CollectorDeleter, matcher func(*dto.Metric) bool) {
 	var data dto.Metric
+	var toDelete []prometheus.Labels
 
 	for m := range collect(c) {
 		if err := m.Write(&data); err != nil {
 			continue
 		}
 		if matcher(&data) {
-			defer c.Delete(makeLabels(data.Label)) // nolint: staticcheck
+			toDelete = append(toDelete, makeLabels(data.Label))
 		}
 	}
+
+	for _, labels := range toDelete {
+		c.Delete(labels)
+	}
 }
 
 const unspecifiedValue = float64(-1)

diff --git a/pkg/rclone/rcserver/rc.go b/pkg/rclone/rcserver/rc.go
@@ -189,7 +189,7 @@ type fileFields struct {
 }
 
 type transFields struct {
-	Transferred []accounting.TransferSnapshot
+	Transferred []accounting.TransferSnapshot `mapstructure:"transferred"`
 }
 
 func aggregateJobInfo(jobParam, statsParam, transParam rc.Params) jobProgress {
@@ -555,15 +555,15 @@ func rcChunkedList(ctx context.Context, in rc.Params) (out rc.Params, err error)
 
 	ctx, cfg := filter.AddConfig(ctx)
 	if newest {
-		if err = cfg.Add(false, VersionedFileRegex); err != nil {
+		if err := cfg.Add(false, VersionedFileRegex); err != nil {
 			return nil, err
 		}
 	}
 	if versioned {
-		if err = cfg.Add(true, VersionedFileRegex); err != nil {
+		if err := cfg.Add(true, VersionedFileRegex); err != nil {
 			return nil, err
 		}
-		if err = cfg.Add(false, `{**}`); err != nil {
+		if err := cfg.Add(false, `{**}`); err != nil {
 			return nil, err
 		}
 	}

diff --git a/pkg/scheduler/listener.go b/pkg/scheduler/listener.go
@@ -10,8 +10,8 @@ import (
 // Listener specifies pluggable hooks for scheduler events.
 // Parametrized by scheduler key type.
 type Listener[K comparable] interface {
-	OnSchedulerStart(ctx context.Context)
-	OnSchedulerStop(ctx context.Context)
+	OnSchedulerStart(context.Context)
+	OnSchedulerStop(context.Context)
 	OnRunStart(ctx *RunContext[K])
 	OnRunSuccess(ctx *RunContext[K])
 	OnRunStop(ctx *RunContext[K])
@@ -28,46 +28,46 @@ type Listener[K comparable] interface {
 
 type nopListener[K comparable] struct{}
 
-func (l nopListener[_]) OnSchedulerStart(ctx context.Context) {
+func (l nopListener[_]) OnSchedulerStart(context.Context) {
 }
 
-func (l nopListener[_]) OnSchedulerStop(ctx context.Context) {
+func (l nopListener[_]) OnSchedulerStop(context.Context) {
 }
 
-func (l nopListener[K]) OnRunStart(ctx *RunContext[K]) {
+func (l nopListener[K]) OnRunStart(*RunContext[K]) {
 }
 
-func (l nopListener[K]) OnRunSuccess(ctx *RunContext[K]) {
+func (l nopListener[K]) OnRunSuccess(*RunContext[K]) {
 }
 
-func (l nopListener[K]) OnRunStop(ctx *RunContext[K]) {
+func (l nopListener[K]) OnRunStop(*RunContext[K]) {
 }
 
-func (l nopListener[K]) OnRunWindowEnd(ctx *RunContext[K]) {
+func (l nopListener[K]) OnRunWindowEnd(*RunContext[K]) {
 }
 
-func (l nopListener[K]) OnRunError(ctx *RunContext[K], err error) {
+func (l nopListener[K]) OnRunError(*RunContext[K], error) {
 }
 
-func (l nopListener[K]) OnSchedule(ctx context.Context, key K, begin, end time.Time, retno int8) {
+func (l nopListener[K]) OnSchedule(_ context.Context, _ K, _, _ time.Time, _ int8) {
 }
 
-func (l nopListener[K]) OnUnschedule(ctx context.Context, key K) {
+func (l nopListener[K]) OnUnschedule(context.Context, K) {
 }
 
-func (l nopListener[K]) OnTrigger(ctx context.Context, key K, success bool) {
+func (l nopListener[K]) OnTrigger(context.Context, K, bool) {
 }
 
-func (l nopListener[K]) OnStop(ctx context.Context, key K) {
+func (l nopListener[K]) OnStop(context.Context, K) {
 }
 
-func (l nopListener[K]) OnRetryBackoff(ctx context.Context, key K, backoff time.Duration, retno int8) {
+func (l nopListener[K]) OnRetryBackoff(context.Context, K, time.Duration, int8) {
 }
 
-func (l nopListener[K]) OnNoTrigger(ctx context.Context, key K) {
+func (l nopListener[K]) OnNoTrigger(context.Context, K) {
 }
 
-func (l nopListener[K]) OnSleep(ctx context.Context, key K, d time.Duration) {
+func (l nopListener[K]) OnSleep(context.Context, K, time.Duration) {
 }
 
 // NopListener returns a Listener implementation that has no effects.