From 113273b649f782279de89b3e8fd4b5d521577609 Mon Sep 17 00:00:00 2001 From: zyy17 Date: Mon, 22 Jan 2024 15:56:51 +0800 Subject: [PATCH] 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 | 8 +- .golangci.yaml | 99 +++++++++++++++++++ Makefile | 21 ++-- apis/v1alpha1/defaulting.go | 6 +- controllers/greptimedbcluster/controller.go | 6 +- .../greptimedbcluster/deployers/meta.go | 2 +- controllers/greptimedbcluster/suite_test.go | 2 +- .../greptimedbstandalone/controller.go | 6 +- pkg/dbconfig/datanode_config.go | 2 +- pkg/dbconfig/dbconfig.go | 4 +- pkg/dbconfig/frontend_config.go | 2 +- pkg/dbconfig/metasrv_config.go | 2 +- pkg/dbconfig/standalone_config.go | 4 +- 13 files changed, 131 insertions(+), 33 deletions(-) create mode 100644 .golangci.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f88f6075..c6fac8ae 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..c6fa90ca --- /dev/null +++ b/.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 diff --git a/Makefile b/Makefile index dee99461..bc5e6b44 100644 --- a/Makefile +++ b/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\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\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) diff --git a/apis/v1alpha1/defaulting.go b/apis/v1alpha1/defaulting.go index df246aaa..7766041f 100644 --- a/apis/v1alpha1/defaulting.go +++ b/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{ diff --git a/controllers/greptimedbcluster/controller.go b/controllers/greptimedbcluster/controller.go index a570ac07..9a50350f 100644 --- a/controllers/greptimedbcluster/controller.go +++ b/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 } } diff --git a/controllers/greptimedbcluster/deployers/meta.go b/controllers/greptimedbcluster/deployers/meta.go index a8f90bc9..3e024b5b 100644 --- a/controllers/greptimedbcluster/deployers/meta.go +++ b/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" diff --git a/controllers/greptimedbcluster/suite_test.go b/controllers/greptimedbcluster/suite_test.go index b062754d..47b03229 100644 --- a/controllers/greptimedbcluster/suite_test.go +++ b/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" diff --git a/controllers/greptimedbstandalone/controller.go b/controllers/greptimedbstandalone/controller.go index 1dd88130..b41b2614 100644 --- a/controllers/greptimedbstandalone/controller.go +++ b/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 } } diff --git a/pkg/dbconfig/datanode_config.go b/pkg/dbconfig/datanode_config.go index 16e4ca1d..86f7d739 100644 --- a/pkg/dbconfig/datanode_config.go +++ b/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 } diff --git a/pkg/dbconfig/dbconfig.go b/pkg/dbconfig/dbconfig.go index 6d18dca3..abd7f21e 100644 --- a/pkg/dbconfig/dbconfig.go +++ b/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. diff --git a/pkg/dbconfig/frontend_config.go b/pkg/dbconfig/frontend_config.go index 22dbe142..90b38cec 100644 --- a/pkg/dbconfig/frontend_config.go +++ b/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 } diff --git a/pkg/dbconfig/metasrv_config.go b/pkg/dbconfig/metasrv_config.go index b75fa054..20a73055 100644 --- a/pkg/dbconfig/metasrv_config.go +++ b/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 } diff --git a/pkg/dbconfig/standalone_config.go b/pkg/dbconfig/standalone_config.go index e17f3464..4813a327 100644 --- a/pkg/dbconfig/standalone_config.go +++ b/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 {