[Cleanup] Linter code cleaning (#718)

Author: informalict

.travis.yml

@@ -13,6 +13,6 @@ install:
   - make init
 
 script:
-  - make license-verify fmt-verify
+  - make license-verify fmt-verify linter
   - make run-unit-tests
   - make bin

Makefile

@@ -138,9 +138,9 @@ ifdef VERBOSE
 	TESTVERBOSEOPTIONS := -v
 endif
 
+EXCLUDE_DIRS := tests vendor .gobuild deps tools
 SOURCES_QUERY := find $(SRCDIR) -name '*.go' -type f -not -path '$(SRCDIR)/tests/*' -not -path '$(SRCDIR)/vendor/*' -not -path '$(SRCDIR)/.gobuild/*' -not -path '$(SRCDIR)/deps/*' -not -path '$(SRCDIR)/tools/*'
 SOURCES := $(shell $(SOURCES_QUERY))
-SOURCES_PACKAGES := $(shell $(SOURCES_QUERY) -exec dirname {} \; | sort | uniq)
 DASHBOARDSOURCES := $(shell find $(DASHBOARDDIR)/src -name '*.js' -not -path './test/*') $(DASHBOARDDIR)/package.json
 
 ifndef ARANGOSYNCSRCDIR
@@ -174,10 +174,8 @@ allall: all
 # Tip: Run `eval $(minikube docker-env)` before calling make if you're developing on minikube.
 #
 
-GOLANGCI_ENABLED=deadcode gocyclo golint varcheck structcheck maligned errcheck \
-                 ineffassign interfacer unconvert goconst \
-                 megacheck
-
+GOLANGCI_ENABLED=deadcode govet ineffassign staticcheck structcheck typecheck unconvert unparam unused varcheck
+#GOLANGCI_ENABLED=gocyclo goconst golint maligned errcheck interfacer megacheck
 #GOLANGCI_ENABLED+=dupl - disable dupl check
 
 .PHONY: license-verify
@@ -201,11 +199,10 @@ fmt-verify: license-verify
 	@if [ X"$$(go run golang.org/x/tools/cmd/goimports -l $(SOURCES) | wc -l)" != X"0" ]; then echo ">> Style errors"; go run golang.org/x/tools/cmd/goimports -l $(SOURCES); exit 1; fi
 
 .PHONY: linter
-linter: fmt
-	@golangci-lint run --no-config --issues-exit-code=1 --deadline=30m --disable-all \
-	                  $(foreach MODE,$(GOLANGCI_ENABLED),--enable $(MODE) ) \
-	                  --exclude-use-default=false \
-	                  $(SOURCES_PACKAGES)
+linter:
+	$(GOPATH)/bin/golangci-lint run --no-config --issues-exit-code=1 --deadline=30m --exclude-use-default=false \
+	--disable-all $(foreach EXCLUDE_DIR,$(EXCLUDE_DIRS),--skip-dirs $(EXCLUDE_DIR)) \
+	$(foreach MODE,$(GOLANGCI_ENABLED),--enable $(MODE)) ./...
 
 .PHONY: build
 build: docker manifests
@@ -599,6 +596,8 @@ init: tools update-generated $(GHRELEASE) $(RELEASE) $(TESTBIN) $(BIN) vendor
 
 .PHONY: tools
 tools: update-vendor
+	@echo ">> Fetching golangci-lint linter"
+	@go install github.com/golangci/golangci-lint/cmd/golangci-lint
 	@echo ">> Fetching goimports"
 	@go get -u golang.org/x/tools/cmd/goimports
 	@echo ">> Fetching license check"
@@ -653,4 +652,4 @@ synchronize-v2alpha1-with-v1:
 	@for file in $$(find "$(ROOT)/pkg/apis/deployment/v1/" -type f -exec realpath --relative-to "$(ROOT)/pkg/apis/deployment/v1/" {} \;); do cat "$(ROOT)/pkg/apis/deployment/v1/$${file}" | sed "s#package v1#package v2alpha1#g" | sed 's#ArangoDeploymentVersion = "v1"#ArangoDeploymentVersion = "v2alpha1"#g' > "$(ROOT)/pkg/apis/deployment/v2alpha1/$${file}"; done
 	@make update-generated
 	@make set-deployment-api-version-v2alpha1 bin
-	@make set-deployment-api-version-v1 bin
+	@make set-deployment-api-version-v1 bin

pkg/apis/deployment/v1/timeouts.go

@@ -28,10 +28,6 @@ import (
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	addMemberTimeout = time.Minute * 5
-)
-
 type Timeouts struct {
 	AddMember *Timeout `json:"addMember,omitempty"`
 }

pkg/apis/deployment/v2alpha1/timeouts.go

@@ -28,10 +28,6 @@ import (
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	addMemberTimeout = time.Minute * 5
-)
-
 type Timeouts struct {
 	AddMember *Timeout `json:"addMember,omitempty"`
 }

pkg/backup/handlers/arango/backup/backup_suite_test.go

@@ -264,7 +264,6 @@ func wrapperProgressMissing(t *testing.T, state state.State) {
 		newObj := refreshArangoBackup(t, handler, obj)
 		require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateFailed)
 
-		require.Equal(t, newObj.Status.Message, createStateMessage(state, backupApi.ArangoBackupStateFailed, fmt.Sprintf("missing field .status.backup")))
-
+		require.Equal(t, newObj.Status.Message, createStateMessage(state, backupApi.ArangoBackupStateFailed, "missing field .status.backup"))
 	})
 }

pkg/backup/handlers/arango/backup/finalizer.go

@@ -175,15 +175,15 @@ func hasFinalizers(backup *backupApi.ArangoBackup) bool {
 	}
 
 	for _, finalizer := range backupApi.FinalizersArangoBackup {
-		if !hasFinalizer(backup, finalizer) {
+		if !hasFinalizer(finalizer) {
 			return false
 		}
 	}
 
 	return true
 }
 
-func hasFinalizer(backup *backupApi.ArangoBackup, finalizer string) bool {
+func hasFinalizer(finalizer string) bool {
 	if backupApi.FinalizersArangoBackup == nil {
 		return false
 	}
@@ -209,7 +209,7 @@ func appendFinalizers(backup *backupApi.ArangoBackup) []string {
 	old := backup.Finalizers
 
 	for _, finalizer := range backupApi.FinalizersArangoBackup {
-		if !hasFinalizer(backup, finalizer) {
+		if !hasFinalizer(finalizer) {
 			old = append(old, finalizer)
 		}
 	}

pkg/backup/handlers/arango/backup/status.go

@@ -118,7 +118,7 @@ func setFailedState(backup *backupApi.ArangoBackup, err error) (*backupApi.Arang
 		updateStatusAvailable(false))
 }
 
-func createStateMessage(from, to state.State, message string) string {
+func createStateMessage(from, to state.State, message string) string { // nolint:unparam
 	return fmt.Sprintf("Transiting from %s to %s: %s", from, to, message)
 }
 

pkg/backup/handlers/arango/policy/handler.go

@@ -74,11 +74,7 @@ func (h *handler) Handle(item operation.Item) error {
 		return err
 	}
 
-	status, err := h.processBackupPolicy(policy.DeepCopy())
-	if err != nil {
-		return err
-	}
-
+	status := h.processBackupPolicy(policy.DeepCopy())
 	// Nothing to update, objects are equal
 	if reflect.DeepEqual(policy.Status, status) {
 		return nil
@@ -94,13 +90,13 @@ func (h *handler) Handle(item operation.Item) error {
 	return nil
 }
 
-func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (backupApi.ArangoBackupPolicyStatus, error) {
+func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) backupApi.ArangoBackupPolicyStatus {
 	if err := policy.Validate(); err != nil {
 		h.eventRecorder.Warning(policy, policyError, "Policy Error: %s", err.Error())
 
 		return backupApi.ArangoBackupPolicyStatus{
 			Message: fmt.Sprintf("Validation error: %s", err.Error()),
-		}, nil
+		}
 	}
 
 	now := time.Now()
@@ -111,7 +107,7 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 
 		return backupApi.ArangoBackupPolicyStatus{
 			Message: fmt.Sprintf("error while parsing expr: %s", err.Error()),
-		}, nil
+		}
 	}
 
 	if policy.Status.Scheduled.IsZero() {
@@ -121,7 +117,7 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 			Scheduled: meta.Time{
 				Time: next,
 			},
-		}, nil
+		}
 	}
 
 	// Check if schedule is required
@@ -135,10 +131,10 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 				Scheduled: meta.Time{
 					Time: next,
 				},
-			}, nil
+			}
 		}
 
-		return policy.Status, nil
+		return policy.Status
 	}
 
 	// Schedule new deployments
@@ -159,7 +155,7 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 		return backupApi.ArangoBackupPolicyStatus{
 			Scheduled: policy.Status.Scheduled,
 			Message:   fmt.Sprintf("deployments listing failed: %s", err.Error()),
-		}, nil
+		}
 	}
 
 	for _, deployment := range deployments.Items {
@@ -171,7 +167,7 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 			return backupApi.ArangoBackupPolicyStatus{
 				Scheduled: policy.Status.Scheduled,
 				Message:   fmt.Sprintf("backup creation failed: %s", err.Error()),
-			}, nil
+			}
 		}
 
 		h.eventRecorder.Normal(policy, backupCreated, "Created ArangoBackup: %s/%s", b.Namespace, b.Name)
@@ -185,7 +181,7 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 		Scheduled: meta.Time{
 			Time: next,
 		},
-	}, nil
+	}
 }
 
 func (*handler) CanBeHandled(item operation.Item) bool {

pkg/backup/handlers/arango/policy/handler_suite_test.go

@@ -69,7 +69,7 @@ func newItem(o operation.Operation, namespace, name string) operation.Item {
 	}
 }
 
-func newItemFromBackupPolicy(operation operation.Operation, policy *backupApi.ArangoBackupPolicy) operation.Item {
+func newItemFromBackupPolicy(operation operation.Operation, policy *backupApi.ArangoBackupPolicy) operation.Item { // nolint:unparam
 	return newItem(operation, policy.Namespace, policy.Name)
 }
 

pkg/backup/operator/operator_suite_test.go

@@ -102,8 +102,8 @@ func mockSimpleObject(name string, canBeHandled bool) (Handler, chan operation.I
 	})
 }
 
-func waitForItems(t *testing.T, i <-chan operation.Item, expectedSize int, timeout time.Duration) []operation.Item {
-	tmout := time.NewTimer(timeout)
+func waitForItems(t *testing.T, i <-chan operation.Item, expectedSize int) []operation.Item {
+	tmout := time.NewTimer(time.Second)
 	defer tmout.Stop()
 	received := make([]operation.Item, 0, expectedSize)
 	for {

pkg/backup/operator/operator_test.go

@@ -79,7 +79,7 @@ func Test_Operator_InformerProcessing(t *testing.T) {
 	}
 
 	// Assert
-	res := waitForItems(t, i, size, time.Second)
+	res := waitForItems(t, i, size)
 	assert.Len(t, res, size)
 
 	time.Sleep(50 * time.Millisecond)
@@ -140,7 +140,7 @@ func Test_Operator_MultipleInformers(t *testing.T) {
 	}
 
 	// Assert
-	res := waitForItems(t, i, size*2, time.Second)
+	res := waitForItems(t, i, size*2)
 	assert.Len(t, res, size*2)
 
 	time.Sleep(50 * time.Millisecond)
@@ -200,7 +200,7 @@ func Test_Operator_MultipleInformers_IgnoredTypes(t *testing.T) {
 	}
 
 	// Assert
-	res := waitForItems(t, i, size, time.Second)
+	res := waitForItems(t, i, size)
 	assert.Len(t, res, size)
 
 	time.Sleep(50 * time.Millisecond)
@@ -300,10 +300,10 @@ func Test_Operator_MultipleInformers_MultipleHandlers(t *testing.T) {
 	}
 
 	// Assert
-	assert.Len(t, waitForItems(t, ip, size, time.Second), size)
-	assert.Len(t, waitForItems(t, in, size, time.Second), size)
-	assert.Len(t, waitForItems(t, is, size, time.Second), size)
-	assert.Len(t, waitForItems(t, id, size, time.Second), size)
+	assert.Len(t, waitForItems(t, ip, size), size)
+	assert.Len(t, waitForItems(t, in, size), size)
+	assert.Len(t, waitForItems(t, is, size), size)
+	assert.Len(t, waitForItems(t, id, size), size)
 
 	time.Sleep(50 * time.Millisecond)
 	assert.Len(t, ip, 0)
@@ -358,7 +358,7 @@ func Test_Operator_InformerProcessing_Namespaced(t *testing.T) {
 	}
 
 	// Assert
-	res := waitForItems(t, i, 1, time.Second)
+	res := waitForItems(t, i, 1)
 	assert.Len(t, res, 1)
 
 	time.Sleep(50 * time.Millisecond)

pkg/deployment/client/client_cache.go

@@ -69,7 +69,7 @@ func (cc *cache) extendHost(host string) string {
 	return scheme + "://" + net.JoinHostPort(host, strconv.Itoa(k8sutil.ArangoPort))
 }
 
-func (cc *cache) getClient(ctx context.Context, group api.ServerGroup, id string) (driver.Client, error) {
+func (cc *cache) getClient(group api.ServerGroup, id string) (driver.Client, error) {
 	m, _, _ := cc.apiObjectGetter().Status.Members.ElementByID(id)
 
 	c, err := cc.factory.Client(cc.extendHost(m.GetEndpoint(k8sutil.CreatePodDNSName(cc.apiObjectGetter(), group.AsRole(), id))))
@@ -80,15 +80,15 @@ func (cc *cache) getClient(ctx context.Context, group api.ServerGroup, id string
 }
 
 func (cc *cache) get(ctx context.Context, group api.ServerGroup, id string) (driver.Client, error) {
-	client, err := cc.getClient(ctx, group, id)
+	client, err := cc.getClient(group, id)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
 
 	if _, err := client.Version(ctx); err == nil {
 		return client, nil
 	} else if driver.IsUnauthorized(err) {
-		return cc.getClient(ctx, group, id)
+		return cc.getClient(group, id)
 	} else {
 		return client, nil
 	}
@@ -103,7 +103,7 @@ func (cc *cache) Get(ctx context.Context, group api.ServerGroup, id string) (dri
 	return cc.get(ctx, group, id)
 }
 
-func (cc cache) GetAuth() conn.Auth {
+func (cc *cache) GetAuth() conn.Auth {
 	return cc.factory.GetAuth()
 }