Skip to content

Commit

Permalink
drop retool, replace gometalinter with golangci-lint, and fix spellin…
Browse files Browse the repository at this point in the history
…gs (pingcap#3005)

* drop retool, replace gometalinter with golangci-lint, and fix spellings

* fix verify scripts

* increase golangci-lint timeout
  • Loading branch information
cofyc authored Jul 23, 2020
1 parent 80f74d7 commit 32e1f58
Show file tree
Hide file tree
Showing 38 changed files with 828 additions and 237 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ coverage.out

# retool. We could check this in like a vendor
# But it is just tools that can be installed with make setup
/_tools/
vendor
tests/e2e/e2e.test
.orig
Expand Down
15 changes: 15 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
run:
timeout: 5m

linters:
enable:
- gofmt
disable:
# Enable once related errors are fixed or ignored explicitly
- staticcheck
- unused
- gosimple
- structcheck
- errcheck
- deadcode
- varcheck
4 changes: 2 additions & 2 deletions CHANGELOG-1.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ For v1.0.x users, refer to [Upgrade TiDB Operator](https://pingcap.com/docs/tidb
For the `tidb-cluster` chart, we already have the `timezone` option (`UTC` by default). If the user does not change it to a different value (for example, `Asia/Shanghai`), none of the Pods will be recreated.
If the user changes it to another value (for example, `Aisa/Shanghai`), all the related Pods (add a `TZ` env) will be recreated, namely rolling updated.

The related Pods include `pump`, `drainer`, `dicovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.
The related Pods include `pump`, `drainer`, `discovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.

All images' time zone maintained by TiDB Operator is `UTC`. If you use your own images, you need to make sure that the time zone inside your images is `UTC`.

Expand Down Expand Up @@ -275,7 +275,7 @@ This is a pre-release of `v1.1.0`, which focuses on the usability, extensibility

Regarding other charts, we don't have a `timezone` option in their `values.yaml`. We add the `timezone` option in this PR. No matter whether the user uses the old `values.yaml` or the new `values.yaml`, all the related Pods (add a `TZ` env) will not be recreated (rolling update).

The related Pods include `pump`, `drainer`, `dicovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.
The related Pods include `pump`, `drainer`, `discovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.

All images' time zone maintained by `tidb-operator` is `UTC`. If you use your own images, you need to make sure that the time zone inside your images is `UTC`.

Expand Down
6 changes: 3 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ ACTION REQUIRED: This release adds the `timezone` support for [all charts](https

For existing TiDB clusters. If the `timezone` in `tidb-cluster/values.yaml` has been customized to other timezones instead of the default `UTC`, then upgrading tidb-operator will trigger a rolling update for the related pods.

The related pods include `pump`, `drainer`, `dicovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.
The related pods include `pump`, `drainer`, `discovery`, `monitor`, `scheduled backup`, `tidb-initializer`, and `tikv-importer`.

The time zone for all images maintained by `tidb-operator` should be `UTC`. If you use your own images, you need to make sure that the corresponding time zones are `UTC`.

Expand Down Expand Up @@ -717,13 +717,13 @@ Please check [cluster configuration](https://pingcap.com/docs/v3.0/tidb-in-kuber
- fix dind doc ([#352](https://github.com/pingcap/tidb-operator/pull/352))
- Add additionalPrintColumns for TidbCluster CRD ([#361](https://github.com/pingcap/tidb-operator/pull/361))
- refactor stability main function ([#363](https://github.com/pingcap/tidb-operator/pull/363))
- enable admin privelege for prom ([#360](https://github.com/pingcap/tidb-operator/pull/360))
- enable admin privilege for prom ([#360](https://github.com/pingcap/tidb-operator/pull/360))
- Updated Readme with New Info ([#365](https://github.com/pingcap/tidb-operator/pull/365))
- Build CLI ([#357](https://github.com/pingcap/tidb-operator/pull/357))
- add extraLabels variable in tidb-cluster chart ([#373](https://github.com/pingcap/tidb-operator/pull/373))
- fix tikv failover ([#368](https://github.com/pingcap/tidb-operator/pull/368))
- Separate and ensure setup before e2e-build ([#375](https://github.com/pingcap/tidb-operator/pull/375))
- Fix codegen.sh and lock related depedencies ([#371](https://github.com/pingcap/tidb-operator/pull/371))
- Fix codegen.sh and lock related dependencies ([#371](https://github.com/pingcap/tidb-operator/pull/371))
- stability: add sst-file-corruption case ([#382](https://github.com/pingcap/tidb-operator/pull/382))
- use release name as default clusterName ([#354](https://github.com/pingcap/tidb-operator/pull/354))
- Add util class to support to add annotations to Grafana ([#378](https://github.com/pingcap/tidb-operator/pull/378))
Expand Down
76 changes: 8 additions & 68 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ GO_BUILD := $(GO) build -trimpath
DOCKER_REGISTRY ?= localhost:5000
DOCKER_REPO ?= ${DOCKER_REGISTRY}/pingcap
IMAGE_TAG ?= latest
PACKAGE_LIST := go list ./... | grep -vE "client/(clientset|informers|listers)"
PACKAGE_DIRECTORIES := $(PACKAGE_LIST) | sed 's|github.com/pingcap/tidb-operator/||'
FILES := $$(find $$($(PACKAGE_DIRECTORIES)) -name "*.go")
FAIL_ON_STDOUT := awk '{ print } END { if (NR > 0) { exit 1 } }'
TEST_COVER_PACKAGES:=go list ./cmd/backup-manager/app/... ./pkg/... | grep -vE "pkg/client" | grep -vE "pkg/tkctl" | grep -vE "pkg/apis" | sed 's|github.com/pingcap/tidb-operator/|./|' | tr '\n' ','

default: build
Expand Down Expand Up @@ -132,77 +128,21 @@ test:
@go test ./cmd/backup-manager/app/... ./pkg/... && echo "\nUnit tests run successfully!"
endif

check-all: lint check-static check-shadow check-gosec staticcheck errcheck

check-setup:
@which retool >/dev/null 2>&1 || GO111MODULE=off go get github.com/twitchtv/retool
@GO111MODULE=off retool sync

check: check-setup lint tidy check-static check-codegen check-terraform check-boilerplate check-openapi-spec check-crd-groups

check-static:
@ # Not running vet and fmt through metalinter because it ends up looking at vendor
@echo "gofmt checking"
gofmt -s -l -w $(FILES) 2>&1| $(FAIL_ON_STDOUT)
@echo "go vet check"
@go vet -all $$($(PACKAGE_LIST)) 2>&1
@echo "mispell and ineffassign checking"
CGO_ENABLED=0 retool do gometalinter.v2 --disable-all \
--enable misspell \
--enable ineffassign \
$$($(PACKAGE_DIRECTORIES))
@echo "end-of-file checking"
./hack/check-EOF.sh

check-codegen:
./hack/verify-codegen.sh

check-terraform:
./hack/check-terraform.sh
git diff --quiet deploy

check-boilerplate:
./hack/verify-boilerplate.sh

check-openapi-spec:
./hack/verify-openapi-spec.sh

check-crd-groups:
./hack/verify-crd-groups.sh

# TODO: staticcheck is too slow currently
staticcheck:
@echo "gometalinter staticcheck"
CGO_ENABLED=0 retool do gometalinter.v2 --disable-all --deadline 120s \
--enable staticcheck \
$$($(PACKAGE_DIRECTORIES))

# TODO: errcheck is too slow currently
errcheck:
@echo "gometalinter errcheck"
CGO_ENABLED=0 retool do gometalinter.v2 --disable-all --deadline 120s \
--enable errcheck \
$$($(PACKAGE_DIRECTORIES))

# TODO: shadow check fails at the moment
check-shadow:
@echo "go vet shadow checking"
go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
@go vet -vettool=$(which shadow) $$($(PACKAGE_LIST))
ALL_CHECKS = EOF codegen terraform boilerplate openapi-spec crd-groups spelling

check: $(addprefix check-,$(ALL_CHECKS)) lint tidy

check-%:
./hack/verify-$*.sh

lint:
@echo "linting"
CGO_ENABLED=0 retool do revive -formatter friendly -config revive.toml $$($(PACKAGE_LIST))
./hack/verify-lint.sh

tidy:
@echo "go mod tidy"
go mod tidy
git diff -U --exit-code go.mod go.sum

check-gosec:
@echo "security checking"
CGO_ENABLED=0 retool do gosec $$($(PACKAGE_DIRECTORIES))

cli:
$(GO_BUILD) -ldflags '$(LDFLAGS)' -o tkctl cmd/tkctl/main.go

Expand All @@ -219,4 +159,4 @@ debug-build-docker: debug-build
debug-build:
$(GO_BUILD) -ldflags '$(LDFLAGS)' -o misc/images/debug-launcher/bin/debug-launcher misc/cmd/debug-launcher/main.go

.PHONY: check check-setup check-all build e2e-build debug-build cli e2e test docker e2e-docker debug-build-docker
.PHONY: check check-setup build e2e-build debug-build cli e2e test docker e2e-docker debug-build-docker
2 changes: 1 addition & 1 deletion charts/tidb-cluster/templates/config/_drainer-config.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ compressor = ""
[syncer]

# Assume the upstream sql-mode.
# If this is set , will use the same sql-mode to parse DDL statment, and set the same sql-mode at downstream when db-type is mysql.
# If this is set , will use the same sql-mode to parse DDL statement, and set the same sql-mode at downstream when db-type is mysql.
# The default value will not set any sql-mode.
# sql-mode = "STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION"

Expand Down
2 changes: 1 addition & 1 deletion charts/tidb-cluster/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ binlog:
workerCount: 16
# the interval time (in seconds) of detect pumps' status (default 10)
detectInterval: 10
# disbale detect causality
# disable detect causality
disableDetect: false
# disable dispatching sqls that in one same binlog; if set true, work-count and txn-batch would be useless
disableDispatch: false
Expand Down
2 changes: 1 addition & 1 deletion charts/tidb-drainer/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ logLevel: info
# refer to https://kubernetes.io/docs/concepts/storage/storage-classes
storageClassName: local-storage
storage: 10Gi
# disbale detect causality
# disable detect causality
disableDetect: false
# if drainer donesn't have checkpoint, use initial commitTS to initial checkpoint
initialCommitTs: "-1"
Expand Down
6 changes: 3 additions & 3 deletions ci/aws-clean-eks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function delete_security_group() {
done
aws ec2 delete-security-group --group-id "$sgId"
if [ $? -eq 0 ]; then
echo "info: succesfully deleted security group '$sgId'"
echo "info: successfully deleted security group '$sgId'"
else
echo "error: failed to deleted security group '$sgId'"
fi
Expand All @@ -47,7 +47,7 @@ function delete_vpc() {
done
aws ec2 delete-vpc --vpc-id "$vpcId"
if [ $? -eq 0 ]; then
echo "info: succesfully deleted vpc '$vpcId'"
echo "info: successfully deleted vpc '$vpcId'"
else
echo "error: failed to deleted vpc '$vpcId'"
fi
Expand Down Expand Up @@ -130,7 +130,7 @@ for CLUSTER in $@; do
echo "info: start to clean eks test cluster '$CLUSTER'"
clean_eks "$CLUSTER"
if [ $? -eq 0 ]; then
echo "info: succesfully cleaned the eks test cluster '$CLUSTER'"
echo "info: successfully cleaned the eks test cluster '$CLUSTER'"
else
echo "fatal: failed to clean the eks test cluster '$CLUSTER'"
exit 1
Expand Down
4 changes: 2 additions & 2 deletions ci/pull_e2e_kind.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Jenkins pipeline for e2e kind job.
//
// We uses ghprb plugin to build pull requests and report results. Some special
// environment variables will be avaiable for jobs that are triggered by GitHub
// environment variables will be available for jobs that are triggered by GitHub
// Pull Request events.
//
// - ghprbActualCommit
Expand Down Expand Up @@ -235,7 +235,7 @@ try {

timeout (time: 2, unit: 'HOURS') {
// use fixed label, so we can reuse previous workers
// increase verison in pod label when we update pod template
// increase version in pod label when we update pod template
def buildPodLabel = "tidb-operator-build-v1"
def resources = [
requests: [
Expand Down
2 changes: 1 addition & 1 deletion deploy/aliyun/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ variable "vpc_id" {
}

variable "group_id" {
description = "Security group id, specify this variable to use and exising security group"
description = "Security group id, specify this variable to use and existing security group"
default = ""
}

Expand Down
8 changes: 4 additions & 4 deletions deploy/gcp/create-service-account.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ cred_file=credentials.auto.tfvars

if cred_path=$( echo var.GCP_CREDENTIALS_PATH | terraform console 2>/dev/null ) && [[ $cred_path ]]; then
if ! command -v jq >/dev/null; then
echo "GCP_CREDENTAILS_PATH already set to $cred_path and jq(1) is not installed to ensure it is for project $project" >&2
echo "GCP_CREDENTIALS_PATH already set to $cred_path and jq(1) is not installed to ensure it is for project $project" >&2
exit 1
elif cred_project=$(jq -r .project_id "$cred_path" 2>/dev/null) && [[ $cred_project != "$project" ]]; then
echo "GCP_CREDENTAILS_PATH already set to $cred_path but credentials project $cred_project does not match current project $project" >&2
echo "GCP_CREDENTIALS_PATH already set to $cred_path but credentials project $cred_project does not match current project $project" >&2
exit 1
elif ! [[ -f $cred_path ]]; then
echo "GCP_CREDENTAILS_PATH already set, but $cred_path doesn't exist" >&2
echo "GCP_CREDENTIALS_PATH already set, but $cred_path doesn't exist" >&2
else
echo "GCP_CREDENTAILS_PATH already set to $cred_path for project $project" >&2
echo "GCP_CREDENTIALS_PATH already set to $cred_path for project $project" >&2
exit
fi
fi
Expand Down
6 changes: 3 additions & 3 deletions deploy/gcp/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,17 @@ variable "tikv_instance_type" {}
variable "tidb_instance_type" {}

variable "pd_image_type" {
description = "PD image type, avaiable: UBUNTU/COS"
description = "PD image type, available: UBUNTU/COS"
default = "COS"
}

variable "tidb_image_type" {
description = "TiDB image type, avaiable: UBUNTU/COS"
description = "TiDB image type, available: UBUNTU/COS"
default = "COS"
}

variable "tikv_image_type" {
description = "TiKV image type, avaiable: UBUNTU/COS"
description = "TiKV image type, available: UBUNTU/COS"
default = "COS"
}

Expand Down
2 changes: 1 addition & 1 deletion deploy/modules/aliyun/tidb-cluster/workers.tf
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ resource "alicloud_ess_scaling_group" "workers" {
}
}

# Create the cooresponding auto-scaling configurations
# Create the corresponding auto-scaling configurations
resource "alicloud_ess_scaling_configuration" "workers" {
count = length(local.tidb_cluster_worker_groups)
scaling_group_id = element(alicloud_ess_scaling_group.workers.*.id, count.index)
Expand Down
2 changes: 1 addition & 1 deletion deploy/modules/aliyun/tidb-operator/operator.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
data "template_file" "kubeconfig_filename" {
template = var.kubeconfig_file
vars = {
kubernetes_depedency = alicloud_cs_managed_kubernetes.k8s.client_cert
kubernetes_dependency = alicloud_cs_managed_kubernetes.k8s.client_cert
}
}

Expand Down
4 changes: 2 additions & 2 deletions deploy/modules/aliyun/tidb-operator/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ variable "key_pair_name" {
}

variable "vpc_id" {
description = "VPC id, specify this variable to use an exsiting VPC and the vswitches in the VPC. Note that when using existing vpc, it is recommended to use a existing security group too. Otherwise you have to set vpc_cidr according to the existing VPC settings to get correct in-cluster security rule."
description = "VPC id, specify this variable to use an existing VPC and the vswitches in the VPC. Note that when using existing vpc, it is recommended to use a existing security group too. Otherwise you have to set vpc_cidr according to the existing VPC settings to get correct in-cluster security rule."
default = ""
}

variable "group_id" {
description = "Security group id, specify this variable to use and exising security group"
description = "Security group id, specify this variable to use and existing security group"
default = ""
}

Expand Down
2 changes: 1 addition & 1 deletion deploy/modules/gcp/tidb-cluster/data.tf
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ data "external" "monitor_port" {
locals {
# examples of location: us-central1 (region), us-central1-b (zone), us-central1-c (zone)
cluster_location_args = "%{if length(split("-", var.gke_cluster_location)) == 3}--zone ${var.gke_cluster_location} %{else}--region ${var.gke_cluster_location} %{endif}"
# TODO Update related code when node locations is avaiable in attributes of cluster resource.
# TODO Update related code when node locations is available in attributes of cluster resource.
cmd_get_cluster_locations = <<EOT
gcloud --project ${var.gcp_project} container clusters list --filter='name=${var.gke_cluster_name}' --format='json[no-heading](locations)' ${local.cluster_location_args} | jq '{"locations": (if (. | length) > 0 then .[0].locations | join(",") else "" end) }'
EOT
Expand Down
6 changes: 3 additions & 3 deletions deploy/modules/gcp/tidb-cluster/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ variable "monitor_instance_type" {
}

variable "pd_image_type" {
description = "PD image type, avaiable: UBUNTU/COS"
description = "PD image type, available: UBUNTU/COS"
default = "COS"
}

variable "tidb_image_type" {
description = "TiDB image type, avaiable: UBUNTU/COS"
description = "TiDB image type, available: UBUNTU/COS"
default = "COS"
}

variable "tikv_image_type" {
description = "TiKV image type, avaiable: UBUNTU/COS"
description = "TiKV image type, available: UBUNTU/COS"
default = "COS"
}

Expand Down
2 changes: 1 addition & 1 deletion deploy/modules/share/tidb-cluster-release/outputs.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
locals {
tidb_hostname = var.create ? lookup(data.external.tidb_hostname[0].result, var.service_ingress_key, "empty") : "not_created"
monitor_hostname = var.create ? lookup(data.external.monitor_hostname[0].result, var.service_ingress_key, "emtpy") : "not_created"
monitor_hostname = var.create ? lookup(data.external.monitor_hostname[0].result, var.service_ingress_key, "empty") : "not_created"
}

output "tidb_hostname" {
Expand Down
1 change: 0 additions & 1 deletion docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ You can now edit the code on the `myfeature` branch.
Run following commands to check your code change.

```
$ make check-setup
$ make check
```

Expand Down
2 changes: 1 addition & 1 deletion docs/stability-test-cookbook.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Stability Test Cookbook

> Important notes: this guide is under heavy development and have complicated enviroment pre-requesites, things are ought to change in the future.
> Important notes: this guide is under heavy development and have complicated environment pre-requesites, things are ought to change in the future.
## Run stability test

Expand Down
2 changes: 1 addition & 1 deletion examples/tiflash/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The following steps will create a TiDB cluster with TiFlash deployed and monitor
- TiDB operator `v1.1.0-rc.3` or higher version installed. [Doc](https://pingcap.com/docs/stable/tidb-in-kubernetes/deploy/tidb-operator/)
- Available `StorageClass` configured, and there are enough PVs (by default, 9 PVs are required) of that storageClass:

The availabe `StorageClass` can by checked with the following command:
The available `StorageClass` can by checked with the following command:

```bash
> kubectl get storageclass
Expand Down
2 changes: 1 addition & 1 deletion hack/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ KUBECTL_BIN=$OUTPUT_BIN/kubectl
HELM_BIN=$OUTPUT_BIN/helm
DOCS_BIN=$OUTPUT_BIN/gen-crd-api-reference-docs
#
# Don't ugprade to 2.15.x/2.16.x until this issue
# Don't upgrade to 2.15.x/2.16.x until this issue
# (https://github.com/helm/helm/issues/6361) has been fixed.
#
HELM_VERSION=${HELM_VERSION:-2.9.1}
Expand Down
Loading

0 comments on commit 32e1f58

Please sign in to comment.