Move to golangci-lint

.circleci/config.yml

@@ -3,7 +3,7 @@ version: 2.1
 # https://circleci.com/blog/circleci-hacks-reuse-yaml-in-your-circleci-config-with-yaml/
 defaults: &defaults
   docker:
-    - image: cortexproject/build-image:master-d74af5958
+    - image: quay.io/cortexproject/build-image:update-lint-ae47f740a
   working_directory: /go/src/github.com/cortexproject/cortex
 
 workflows:
@@ -68,6 +68,10 @@ jobs:
     - run:
         name: Lint
         command: make BUILD_IN_CONTAINER=false lint
+    # fails to run everything first time - see https://github.com/golangci/golangci-lint/issues/866
+    - run:
+        name: Lint again
+        command: make BUILD_IN_CONTAINER=false lint
     - run:
         name: Check vendor directory is consistent.
         command: make BUILD_IN_CONTAINER=false mod-check

.errcheck-exclude

@@ -0,0 +1,3 @@
+io/ioutil.WriteFile
+io/ioutil.ReadFile
+(github.com/go-kit/kit/log.Logger).Log

.golangci.yml

@@ -0,0 +1,8 @@
+output:
+  format: line-number
+
+linters-settings:
+  errcheck:
+    # path to a file containing a list of functions to exclude from checking
+    # see https://github.com/kisielk/errcheck#excluding-functions for details
+    exclude: ./.errcheck-exclude

Makefile

@@ -78,27 +78,24 @@ GO_FLAGS := -ldflags "-extldflags \"-static\" -s -w" -tags netgo
 
 ifeq ($(BUILD_IN_CONTAINER),true)
 
+GOVOLUMES=	-v $(shell pwd)/.cache:/go/cache:delegated \
+			-v $(shell pwd)/.pkg:/go/pkg:delegated \
+			-v $(shell pwd):/go/src/github.com/cortexproject/cortex:delegated
+
 exes $(EXES) protos $(PROTO_GOS) lint test shell mod-check check-protos web-build web-pre web-deploy: build-image/$(UPTODATE)
 	@mkdir -p $(shell pwd)/.pkg
 	@mkdir -p $(shell pwd)/.cache
 	@echo
 	@echo ">>>> Entering build container: $@"
-	@$(SUDO) time docker run $(RM) $(TTY) -i \
-		-v $(shell pwd)/.cache:/go/cache \
-		-v $(shell pwd)/.pkg:/go/pkg \
-		-v $(shell pwd):/go/src/github.com/cortexproject/cortex \
-		$(BUILD_IMAGE) $@;
+	@$(SUDO) time docker run $(RM) $(TTY) -i $(GOVOLUMES) $(BUILD_IMAGE) $@;
 
 configs-integration-test: build-image/$(UPTODATE)
 	@mkdir -p $(shell pwd)/.pkg
 	@mkdir -p $(shell pwd)/.cache
 	@DB_CONTAINER="$$(docker run -d -e 'POSTGRES_DB=configs_test' postgres:9.6)"; \
 	echo ; \
 	echo ">>>> Entering build container: $@"; \
-	$(SUDO) docker run $(RM) $(TTY) -i \
-		-v $(shell pwd)/.cache:/go/cache \
-		-v $(shell pwd)/.pkg:/go/pkg \
-		-v $(shell pwd):/go/src/github.com/cortexproject/cortex \
+	$(SUDO) docker run $(RM) $(TTY) -i $(GOVOLUMES) \
 		-v $(shell pwd)/cmd/cortex/migrations:/migrations \
 		--workdir /go/src/github.com/cortexproject/cortex \
 		--link "$$DB_CONTAINER":configs-db.cortex.local \
@@ -121,11 +118,8 @@ protos: $(PROTO_GOS)
 	protoc -I $(GOPATH)/src:./vendor:./$(@D) --gogoslick_out=plugins=grpc,Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,:./$(@D) ./$(patsubst %.pb.go,%.proto,$@)
 
 lint:
-	./tools/lint -notestpackage -novet -ignorespelling queriers -ignorespelling Queriers .
-
-	# -stdmethods=false disables checks for non-standard signatures for methods with familiar names.
-	# This is needed because the Prometheus storage interface requires a non-standard Seek() method.
-	go vet -stdmethods=false ./pkg/...
+	misspell -error docs
+	golangci-lint run --new-from-rev ed7c302fd968 --build-tags netgo --timeout=5m --enable golint --enable misspell --enable gofmt
 
 test:
 	./tools/test -netgo

build-image/Dockerfile

@@ -4,9 +4,6 @@ RUN apt-get update && apt-get install -y curl python-requests python-yaml file j
 RUN curl -sL https://deb.nodesource.com/setup_6.x | sh -
 RUN apt-get install -y nodejs npm && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
 RUN npm install -g postcss-cli autoprefixer
-RUN go clean -i net && \
-	go install -tags netgo std && \
-	go install -race -tags netgo std
 ENV HUGO_VERSION=v0.59.1
 RUN git clone https://github.com/gohugoio/hugo.git --branch ${HUGO_VERSION} --depth 1 && \
 	cd hugo  && go install --tags extended && cd ../ && \
@@ -15,17 +12,13 @@ RUN curl -fsSLo shfmt https://github.com/mvdan/sh/releases/download/v1.3.0/shfmt
 	echo "b1925c2c405458811f0c227266402cf1868b4de529f114722c2e3a5af4ac7bb2  shfmt" | sha256sum -c && \
 	chmod +x shfmt && \
 	mv shfmt /usr/bin
+RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b /usr/bin v1.21.0
 RUN GO111MODULE=on go get -tags netgo \
-		github.com/fzipp/gocyclo \
-		golang.org/x/lint/golint \
-		github.com/kisielk/[email protected] \
 		github.com/client9/misspell/cmd/[email protected] \
 		github.com/golang/protobuf/[email protected] \
 		github.com/gogo/protobuf/[email protected] \
 		github.com/gogo/protobuf/[email protected] && \
 	rm -rf /go/pkg /go/src
-RUN curl -Ls https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 -o $GOPATH/bin/dep && \
-  chmod +x $GOPATH/bin/dep
 
 ENV NODE_PATH=/usr/lib/node_modules
 COPY build.sh /

pkg/configs/configs.go

@@ -203,10 +203,8 @@ func (c RulesConfig) parseV2Formatted() (map[string]rulefmt.RuleGroups, error) {
 
 	for fn, content := range c.Files {
 		rgs, errs := rulefmt.Parse([]byte(content))
-		if errs != nil {
-			for _, err := range errs {
-				return nil, err
-			}
+		for _, err := range errs { // return just the first error, if any
+			return nil, err
 		}
 		ruleMap[fn] = *rgs
 

pkg/cortex/modules.go

@@ -312,7 +312,7 @@ func (t *Cortex) initQueryFrontend(cfg *Config) (err error) {
 func (t *Cortex) stopQueryFrontend() (err error) {
 	t.frontend.Close()
 	if t.cache != nil {
-		t.cache.Stop()
+		_ = t.cache.Stop()
 		t.cache = nil
 	}
 	return

pkg/ingester/ingester_v2.go

@@ -407,12 +407,14 @@ func (i *Ingester) getOrCreateTSDB(userID string, force bool) (*tsdb.DB, error)
 	i.done.Add(1)
 	go func() {
 		defer i.done.Done()
-		runutil.Repeat(i.cfg.TSDBConfig.ShipInterval, i.quit, func() error {
+		if err := runutil.Repeat(i.cfg.TSDBConfig.ShipInterval, i.quit, func() error {
 			if uploaded, err := s.Sync(context.Background()); err != nil {
 				level.Warn(util.Logger).Log("err", err, "uploaded", uploaded)
 			}
 			return nil
-		})
+		}); err != nil {
+			level.Warn(util.Logger).Log("err", err)
+		}
 	}()
 
 	i.TSDBState.dbs[userID] = db

pkg/ring/lifecycler_test.go

@@ -340,7 +340,6 @@ func TestCheckReady(t *testing.T) {
 }
 
 type noopFlushTransferer struct {
-	lifecycler *Lifecycler
 }
 
 func (f *noopFlushTransferer) StopIncomingRequests()                 {}

pkg/ring/model.go

@@ -171,7 +171,7 @@ func (i *IngesterDesc) IsHealthy(op Operation, heartbeatTimeout time.Duration) b
 		healthy = true
 	}
 
-	return healthy && time.Now().Sub(time.Unix(i.Timestamp, 0)) <= heartbeatTimeout
+	return healthy && time.Since(time.Unix(i.Timestamp, 0)) <= heartbeatTimeout
 }
 
 // Merge merges other ring into this one. Returns sub-ring that represents the change,

pkg/ruler/rules/store_test.go

@@ -33,7 +33,7 @@ func Test_ConfigRuleStoreError(t *testing.T) {
 	}
 
 	store := NewConfigRuleStore(mock)
-	_, err := store.ListAllRuleGroups(nil)
+	_, err := store.ListAllRuleGroups(context.Background())
 
 	assert.Equal(t, mock.err, err, "Unexpected error returned")
 }
@@ -52,7 +52,7 @@ func Test_ConfigRuleStoreReturn(t *testing.T) {
 	}
 
 	store := NewConfigRuleStore(mock)
-	rules, _ := store.ListAllRuleGroups(nil)
+	rules, _ := store.ListAllRuleGroups(context.Background())
 
 	assert.Equal(t, 1, len(rules["user"]))
 	assert.Equal(t, id, store.since)
@@ -71,15 +71,15 @@ func Test_ConfigRuleStoreDelete(t *testing.T) {
 	}
 
 	store := NewConfigRuleStore(mock)
-	store.ListAllRuleGroups(nil)
+	_, _ = store.ListAllRuleGroups(context.Background())
 
 	mock.cfgs["user"] = configs.VersionedRulesConfig{
 		ID:        1,
 		Config:    configs.RulesConfig{},
 		DeletedAt: time.Unix(0, 1),
 	}
 
-	rules, _ := store.ListAllRuleGroups(nil)
+	rules, _ := store.ListAllRuleGroups(context.Background())
 
 	assert.Equal(t, 0, len(rules["user"]))
 }
@@ -97,7 +97,7 @@ func Test_ConfigRuleStoreAppend(t *testing.T) {
 	}
 
 	store := NewConfigRuleStore(mock)
-	store.ListAllRuleGroups(nil)
+	_, _ = store.ListAllRuleGroups(context.Background())
 
 	delete(mock.cfgs, "user")
 	mock.cfgs["user2"] = configs.VersionedRulesConfig{
@@ -106,7 +106,7 @@ func Test_ConfigRuleStoreAppend(t *testing.T) {
 		DeletedAt: zeroTime,
 	}
 
-	rules, _ := store.ListAllRuleGroups(nil)
+	rules, _ := store.ListAllRuleGroups(context.Background())
 
 	assert.Equal(t, 2, len(rules))
 }
@@ -134,7 +134,7 @@ func Test_ConfigRuleStoreSinceSet(t *testing.T) {
 	}
 
 	store := NewConfigRuleStore(mock)
-	store.ListAllRuleGroups(nil)
+	_, _ = store.ListAllRuleGroups(context.Background())
 	assert.Equal(t, configs.ID(100), store.since)
 
 	delete(mock.cfgs, "user")
@@ -145,7 +145,7 @@ func Test_ConfigRuleStoreSinceSet(t *testing.T) {
 		DeletedAt: zeroTime,
 	}
 
-	store.ListAllRuleGroups(nil)
+	_, _ = store.ListAllRuleGroups(context.Background())
 	assert.Equal(t, configs.ID(100), store.since)
 
 	mock.cfgs["user2"] = configs.VersionedRulesConfig{
@@ -154,7 +154,7 @@ func Test_ConfigRuleStoreSinceSet(t *testing.T) {
 		DeletedAt: zeroTime,
 	}
 
-	store.ListAllRuleGroups(nil)
+	_, _ = store.ListAllRuleGroups(context.Background())
 	assert.Equal(t, configs.ID(101), store.since)
 }
 

pkg/util/limiter/rate_limiter.go

@@ -99,7 +99,7 @@ func (l *RateLimiter) recheckTenantLimiter(now time.Time, tenantID string) *rate
 	l.tenantsLock.Lock()
 	defer l.tenantsLock.Unlock()
 
-	entry, _ := l.tenants[tenantID]
+	entry := l.tenants[tenantID]
 
 	// We check again if the recheck period elapsed, cause it may
 	// have already been rechecked in the meanwhile.