test: add .golangci.yaml and refactor make lint (#141)

* test(lint): refactor golangci

* test(lint): fix lint errors

* chore(makefile): use 'GOLANGCI_LINT_VERSION'

* ci: change task order

* ci: fix lint errors

.github/workflows/ci.yaml

@@ -32,10 +32,6 @@ jobs:
         with:
           go-version: ${{ env.GO_VERSION }}
 
-      - name: Make lint
-        run: |
-          make lint
-
       - name: Check code generation
         run: |
           make check-code-generation
@@ -44,6 +40,10 @@ jobs:
         run: |
           make
 
+      - name: Make lint
+        run: |
+          make lint
+
   unit-tests:
     name: Run unit tests
     runs-on: ubuntu-22.04

.golangci.yaml

@@ -0,0 +1,99 @@
+# Options for analysis running.
+run:
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 10m
+
+  # The default concurrency value is the number of available CPU.
+  concurrency: 4
+
+  # Which dirs to skip: issues from them won't be reported.
+  # Can use regexp here: `generated.*`, regexp is applied on full path,
+  # including the path prefix if one is set.
+  # Default value is empty list,
+  # but default dirs are skipped independently of this option's value (see skip-dirs-use-default).
+  # "/" will be replaced by current OS file path separator to properly work on Windows.
+  skip-dirs:
+    - bin
+    - docs
+    - examples
+    - hack
+
+# output configuration options.
+output:
+  # Format: colored-line-number|line-number|json|colored-tab|tab|checkstyle|code-climate|junit-xml|github-actions|teamcity
+  #
+  # Multiple can be specified by separating them by comma, output can be provided
+  # for each of them by separating format name and path by colon symbol.
+  # Output path can be either `stdout`, `stderr` or path to the file to write to.
+  # Example: "checkstyle:report.xml,json:stdout,colored-line-number"
+  #
+  # Default: colored-line-number
+  format: colored-line-number
+
+  # Print lines of code with issue.
+  # Default: true
+  print-issued-lines: true
+
+  # Print linter name in the end of issue text.
+  # Default: true
+  print-linter-lines: true
+
+linters:
+  # Disable all linters.
+  disable-all: true
+
+  # Enable specific linter
+  # https://golangci-lint.run/usage/linters/#enabled-by-default
+  enable:
+    # Errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases.
+    - errcheck
+
+    # Linter for Go source code that specializes in simplifying code.
+    - gosimple
+
+    # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string.
+    - govet
+
+    # Detects when assignments to existing variables are not used.
+    - ineffassign
+
+    # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary.
+    # The author of staticcheck doesn't support or approve the use of staticcheck as a library inside golangci-lint.
+    - staticcheck
+
+    # Check import statements are formatted according to the 'goimport' command. Reformat imports in autofix mode.
+    - goimports
+
+    # Checks whether HTTP response body is closed successfully.
+    - bodyclose
+
+    # Provides diagnostics that check for bugs, performance and style issues.
+    # Extensible without recompilation through dynamic rules.
+    # Dynamic rules are written declaratively with AST patterns, filters, report message and optional suggestion.
+    - gocritic
+
+    # Gofmt checks whether code was gofmt-ed. By default, this tool runs with -s option to check for code simplification.
+    - gofmt
+
+    # Finds repeated strings that could be replaced by a constant.
+    - goconst
+
+    # Finds commonly misspelled English words in comments.
+    - misspell
+
+    # Finds naked returns in functions greater than a specified function length.
+    - nakedret
+
+linters-settings:
+  nakedret:
+    # Make an issue if func has more lines of code than this setting, and it has naked returns.
+    # Default: 30
+    max-func-lines: 50
+  goimports:
+    # A comma-separated list of prefixes, which, if set, checks import paths
+    # with the given prefixes are grouped after 3rd-party packages.
+    # Default: ""
+    local-prefixes: github.com/GreptimeTeam/greptimedb-operator
+  linters-settings:
+    min-occurrences: 3

Makefile

@@ -25,7 +25,7 @@ MANIFESTS_DIR = ./manifests
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
 ENVTEST_K8S_VERSION = 1.24.1
 
-GOLANGCI_LINT_VERSION = v1.54.2
+GOLANGCI_LINT_VERSION = v1.55.2
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
@@ -60,7 +60,7 @@ all: build
 
 .PHONY: help
 help: ## Display this help.
-	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf "  \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf "  \033[36m%-25s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
 
 ##@ Development
 
@@ -78,14 +78,6 @@ generate: kustomize controller-gen ## Generate code containing DeepCopy, DeepCop
 fmt: ## Run go fmt against code.
 	go fmt ./...
 
-.PHONY: install-golint
-install-golint: ## Install golint
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}
-
-.PHONY: lint
-lint: install-golint ## Run golint
-	$(GOBIN)/golangci-lint run --timeout 5m0s
-
 .PHONY: check-code-generation
 check-code-generation: ## Check code generation.
 	echo "Checking code generation"
@@ -103,6 +95,10 @@ setup-e2e: ## Setup e2e test environment.
 e2e: setup-e2e ## Run e2e tests.
 	go test -timeout 8m -v ./tests/e2e/... && kind delete clusters greptimedb-operator-e2e
 
+.PHONY: lint
+lint: golangci-lint ## Run lint.
+	$(GOLANGCI_LINT) run -v ./...
+
 .PHONY: test
 test: manifests generate fmt vet envtest ## Run tests.
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test \
@@ -180,6 +176,7 @@ $(LOCALBIN):
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 ENVTEST ?= $(LOCALBIN)/setup-envtest
+GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
 
 ## Tool Versions
 KUSTOMIZE_VERSION ?= v4.5.3
@@ -200,3 +197,7 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+
+.PHONY: golangci-lint
+golangci-lint: ## Install golangci-lint.
+	test -f $(GOLANGCI_LINT) || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(LOCALBIN) $(GOLANGCI_LINT_VERSION)

apis/v1alpha1/defaulting.go

@@ -15,6 +15,7 @@
 package v1alpha1
 
 import (
+	"path"
 	"strings"
 
 	"github.com/imdario/mergo"
@@ -50,6 +51,7 @@ var (
 	defaultDataNodeStorageSize      = "10Gi"
 	defaultDataNodeStorageMountPath = "/data/greptimedb"
 	defaultStorageRetainPolicyType  = StorageRetainPolicyTypeRetain
+	defaultWalDir                   = path.Join(defaultDataNodeStorageMountPath, "wal")
 
 	defaultInitializer = "greptime/greptimedb-initializer:latest"
 )
@@ -139,7 +141,7 @@ func (in *GreptimeDBCluster) SetDefaults() error {
 				StorageSize:         defaultDataNodeStorageSize,
 				MountPath:           defaultDataNodeStorageMountPath,
 				StorageRetainPolicy: defaultStorageRetainPolicyType,
-				WalDir:              defaultDataNodeStorageMountPath + "/wal",
+				WalDir:              defaultWalDir,
 				DataHome:            defaultDataNodeStorageMountPath,
 			},
 		}
@@ -216,7 +218,7 @@ func (in *GreptimeDBStandalone) SetDefaults() error {
 			StorageSize:         defaultDataNodeStorageSize,
 			MountPath:           defaultDataNodeStorageMountPath,
 			StorageRetainPolicy: defaultStorageRetainPolicyType,
-			WalDir:              defaultDataNodeStorageMountPath + "/wal",
+			WalDir:              defaultWalDir,
 			DataHome:            defaultDataNodeStorageMountPath,
 		},
 		Service: &ServiceSpec{

controllers/greptimedbcluster/controller.go

@@ -361,9 +361,7 @@ func (r *Reconciler) setObservedGeneration(cluster *v1alpha1.GreptimeDBCluster)
 	generation := cluster.Generation
 	if cluster.Status.ObservedGeneration == nil {
 		cluster.Status.ObservedGeneration = &generation
-	} else {
-		if *cluster.Status.ObservedGeneration != generation {
-			cluster.Status.ObservedGeneration = &generation
-		}
+	} else if *cluster.Status.ObservedGeneration != generation {
+		cluster.Status.ObservedGeneration = &generation
 	}
 }

controllers/greptimedbcluster/deployers/meta.go

@@ -21,7 +21,7 @@ import (
 	"strings"
 	"time"
 
-	"go.etcd.io/etcd/client/v3"
+	clientv3 "go.etcd.io/etcd/client/v3"
 	"google.golang.org/grpc"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"

controllers/greptimedbcluster/suite_test.go

@@ -24,7 +24,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
-	"go.etcd.io/etcd/client/v3"
+	clientv3 "go.etcd.io/etcd/client/v3"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"

controllers/greptimedbstandalone/controller.go

@@ -327,10 +327,8 @@ func (r *Reconciler) setObservedGeneration(standalone *v1alpha1.GreptimeDBStanda
 	generation := standalone.Generation
 	if standalone.Status.ObservedGeneration == nil {
 		standalone.Status.ObservedGeneration = &generation
-	} else {
-		if *standalone.Status.ObservedGeneration != generation {
-			standalone.Status.ObservedGeneration = &generation
-		}
+	} else if *standalone.Status.ObservedGeneration != generation {
+		standalone.Status.ObservedGeneration = &generation
 	}
 }
 

pkg/dbconfig/datanode_config.go

@@ -96,7 +96,7 @@ func (c *DatanodeConfig) ConfigureByCluster(cluster *v1alpha1.GreptimeDBCluster)
 	return nil
 }
 
-// ConfigureByStandalone is not need to implemenet in cluster mode.
+// ConfigureByStandalone is not need to implement in cluster mode.
 func (c *DatanodeConfig) ConfigureByStandalone(_ *v1alpha1.GreptimeDBStandalone) error {
 	return nil
 }

pkg/dbconfig/dbconfig.go

@@ -70,12 +70,12 @@ func Marshal(config Config) ([]byte, error) {
 		return nil, err
 	}
 
-	ouput, err := setConfig(tree, config)
+	output, err := setConfig(tree, config)
 	if err != nil {
 		return nil, err
 	}
 
-	return []byte(ouput), nil
+	return []byte(output), nil
 }
 
 // FromCluster creates config data from the cluster CRD.

pkg/dbconfig/frontend_config.go

@@ -37,7 +37,7 @@ func (c *FrontendConfig) ConfigureByCluster(cluster *v1alpha1.GreptimeDBCluster)
 	return nil
 }
 
-// ConfigureByStandalone is not need to implemenet in cluster mode.
+// ConfigureByStandalone is not need to implement in cluster mode.
 func (c *FrontendConfig) ConfigureByStandalone(_ *v1alpha1.GreptimeDBStandalone) error {
 	return nil
 }

pkg/dbconfig/metasrv_config.go

@@ -51,7 +51,7 @@ func (c *MetasrvConfig) ConfigureByCluster(cluster *v1alpha1.GreptimeDBCluster)
 	return nil
 }
 
-// ConfigureByStandalone is not need to implemenet in cluster mode.
+// ConfigureByStandalone is not need to implement in cluster mode.
 func (c *MetasrvConfig) ConfigureByStandalone(_ *v1alpha1.GreptimeDBStandalone) error {
 	return nil
 }

pkg/dbconfig/standalone_config.go

@@ -40,12 +40,12 @@ type StandaloneConfig struct {
 	InputConfig string
 }
 
-// ConfigureByCluster is not need to implemenet in standalone mode.
+// ConfigureByCluster is not need to implement in standalone mode.
 func (c *StandaloneConfig) ConfigureByCluster(cluster *v1alpha1.GreptimeDBCluster) error {
 	return nil
 }
 
-// ConfigureByStandalone is not need to implemenet in cluster mode.
+// ConfigureByStandalone is not need to implement in cluster mode.
 func (c *StandaloneConfig) ConfigureByStandalone(standalone *v1alpha1.GreptimeDBStandalone) error {
 	// TODO(zyy17): need to refactor the following code. It's too ugly.
 	if standalone.Spec.ObjectStorageProvider != nil {