From 1018eda27a3303e853f0cce708edd93de7c6c440 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 4 Mar 2019 09:16:31 +0530 Subject: [PATCH 1/4] replace gometalinter with golangci gometalinter is being deprecated in favor of golangci. switching to golangci in ci and make test Signed-off-by: Madhu Rajanna --- .travis.yml | 12 ++-- scripts/golangci.yml | 142 +++++++++++++++++++++++++++++++++++++++++++ scripts/lint-go.sh | 9 +-- 3 files changed, 151 insertions(+), 12 deletions(-) create mode 100644 scripts/golangci.yml diff --git a/.travis.yml b/.travis.yml index 9c4d7f76a..8175a7ef2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,7 @@ go: 1.11.x env: global: - - GO_METALINTER_VERSION="v3.0.0" + - GOLANGCI_VERSION="v1.15.0" - TEST_COVERAGE=stdout - GO_METALINTER_THREADS=1 - GO_COVER_DIR=_output @@ -30,10 +30,10 @@ jobs: - gem install mdl - pip install --user --upgrade pip - pip install --user yamllint - # install gometalinter - - curl -L - "https://raw.githubusercontent.com/alecthomas/gometalinter/"${GO_METALINTER_VERSION}"/scripts/install.sh" - | bash -s -- -b $GOPATH/bin "${GO_METALINTER_VERSION}" + # install golangci-lint + - curl -sf + "https://install.goreleaser.com/github.com/golangci/golangci-lint.sh" + | bash -s -- -b $GOPATH/bin "${GOLANGCI_VERSION}" script: - scripts/lint-text.sh --require-all - scripts/lint-go.sh @@ -49,6 +49,6 @@ jobs: deploy: - provider: script - on: # yamllint disable-line rule:truthy + on: # yamllint disable-line rule:truthy all_branches: true script: ./deploy.sh diff --git a/scripts/golangci.yml b/scripts/golangci.yml new file mode 100644 index 000000000..3675e4b89 --- /dev/null +++ b/scripts/golangci.yml @@ -0,0 +1,142 @@ +--- +# https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml +# This file contains all available configuration options +# with their default values. + +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + deadline: 10m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: true + + # which dirs to skip: they won't be analyzed; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but next dirs are always skipped independently + # from this option's value: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs: + - vendor$ + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + skip-files: + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: colored-line-number + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + +# all available settings of specific linters +linters-settings: + errcheck: + # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: true + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: true + + # path to a file containing a list of functions to exclude from checking + # see https://github.com/kisielk/errcheck#excluding-functions for details + #exclude: /path/to/file.txt + govet: + # report about shadowed variables + check-shadowing: true + golint: + # minimal confidence for issues, default is 0.8 + min-confidence: 0 + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: github.com/ceph/csph-csi + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 20 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + depguard: + list-type: blacklist + include-go-root: false + packages: + - github.com/davecgh/go-spew/spew + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: US + ignore-words: + - someword + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the + # tab-width option + # TODO make line length to 120 char + line-length: 180 + # tab width in spaces. Default to 1. + tab-width: 1 + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + unparam: + # Inspect exported functions, default is false. Set to true if no external program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + +linters: + enable: + - megacheck + - govet + - golint + - stylecheck + - interfacer + - unconvert + - gofmt + - gocyclo + - maligned + - lll + - nakedret + enable-all: false + disable: + - prealloc + disable-all: false + presets: + - bugs + - unused + fast: false diff --git a/scripts/lint-go.sh b/scripts/lint-go.sh index 0f9d49d18..c55112749 100755 --- a/scripts/lint-go.sh +++ b/scripts/lint-go.sh @@ -2,11 +2,8 @@ set -o pipefail -if [[ -x "$(command -v gometalinter)" ]]; then - gometalinter -j "${GO_METALINTER_THREADS:-1}" \ - --sort path --sort line --sort column --deadline=10m \ - --enable=misspell --enable=staticcheck \ - --vendor "${@-./...}" +if [[ -x "$(command -v golangci-lint)" ]]; then + golangci-lint --config=scripts/golangci.yml run ./... -v else - echo "WARNING: gometalinter not found, skipping lint tests" >&2 + echo "WARNING: golangci-lint not found, skipping lint tests" >&2 fi From 8f07c9efcc5132051667ae6333b8c0dfb9026dd9 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 4 Mar 2019 09:40:44 +0530 Subject: [PATCH 2/4] remove unused param from function Signed-off-by: Madhu Rajanna --- pkg/cephfs/nodeserver.go | 2 +- pkg/cephfs/volume.go | 2 +- pkg/cephfs/volumemounter.go | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/cephfs/nodeserver.go b/pkg/cephfs/nodeserver.go index 4c97a80f2..51c44933a 100644 --- a/pkg/cephfs/nodeserver.go +++ b/pkg/cephfs/nodeserver.go @@ -150,7 +150,7 @@ func (*NodeServer) mount(volOptions *volumeOptions, req *csi.NodeStageVolumeRequ klog.V(4).Infof("cephfs: mounting volume %s with %s", volID, m.name()) - if err = m.mount(stagingTargetPath, cr, volOptions, volID); err != nil { + if err = m.mount(stagingTargetPath, cr, volOptions); err != nil { klog.Errorf("failed to mount volume %s: %v", volID, err) return status.Error(codes.Internal, err.Error()) } diff --git a/pkg/cephfs/volume.go b/pkg/cephfs/volume.go index 7b8dea03a..caf9887dc 100644 --- a/pkg/cephfs/volume.go +++ b/pkg/cephfs/volume.go @@ -137,7 +137,7 @@ func mountCephRoot(volID volumeID, volOptions *volumeOptions, adminCr *credentia return fmt.Errorf("failed to create mounter: %v", err) } - if err = m.mount(cephRoot, adminCr, volOptions, volID); err != nil { + if err = m.mount(cephRoot, adminCr, volOptions); err != nil { return fmt.Errorf("error mounting ceph root: %v", err) } diff --git a/pkg/cephfs/volumemounter.go b/pkg/cephfs/volumemounter.go index 035a161a4..6a91afafc 100644 --- a/pkg/cephfs/volumemounter.go +++ b/pkg/cephfs/volumemounter.go @@ -67,7 +67,7 @@ func loadAvailableMounters() error { } type volumeMounter interface { - mount(mountPoint string, cr *credentials, volOptions *volumeOptions, volID volumeID) error + mount(mountPoint string, cr *credentials, volOptions *volumeOptions) error name() string } @@ -111,7 +111,7 @@ func newMounter(volOptions *volumeOptions) (volumeMounter, error) { type fuseMounter struct{} -func mountFuse(mountPoint string, cr *credentials, volOptions *volumeOptions, volID volumeID) error { +func mountFuse(mountPoint string, cr *credentials, volOptions *volumeOptions) error { args := [...]string{ mountPoint, "-m", volOptions.Monitors, @@ -147,19 +147,19 @@ func mountFuse(mountPoint string, cr *credentials, volOptions *volumeOptions, vo return nil } -func (m *fuseMounter) mount(mountPoint string, cr *credentials, volOptions *volumeOptions, volID volumeID) error { +func (m *fuseMounter) mount(mountPoint string, cr *credentials, volOptions *volumeOptions) error { if err := createMountPoint(mountPoint); err != nil { return err } - return mountFuse(mountPoint, cr, volOptions, volID) + return mountFuse(mountPoint, cr, volOptions) } func (m *fuseMounter) name() string { return "Ceph FUSE driver" } type kernelMounter struct{} -func mountKernel(mountPoint string, cr *credentials, volOptions *volumeOptions, volID volumeID) error { +func mountKernel(mountPoint string, cr *credentials, volOptions *volumeOptions) error { if err := execCommandErr("modprobe", "ceph"); err != nil { return err } @@ -172,12 +172,12 @@ func mountKernel(mountPoint string, cr *credentials, volOptions *volumeOptions, ) } -func (m *kernelMounter) mount(mountPoint string, cr *credentials, volOptions *volumeOptions, volID volumeID) error { +func (m *kernelMounter) mount(mountPoint string, cr *credentials, volOptions *volumeOptions) error { if err := createMountPoint(mountPoint); err != nil { return err } - return mountKernel(mountPoint, cr, volOptions, volID) + return mountKernel(mountPoint, cr, volOptions) } func (m *kernelMounter) name() string { return "Ceph kernel client" } From 57cea727fa98e024701371942ae42769c5a01d26 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 4 Mar 2019 09:58:37 +0530 Subject: [PATCH 3/4] Fix yaml lint errors Signed-off-by: Madhu Rajanna --- .travis.yml | 2 +- scripts/golangci.yml | 35 +++++++++++++++++++++++------------ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8175a7ef2..bd1ccc0c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -49,6 +49,6 @@ jobs: deploy: - provider: script - on: # yamllint disable-line rule:truthy + on: # yamllint disable-line rule:truthy all_branches: true script: ./deploy.sh diff --git a/scripts/golangci.yml b/scripts/golangci.yml index 3675e4b89..1aa82525b 100644 --- a/scripts/golangci.yml +++ b/scripts/golangci.yml @@ -33,7 +33,8 @@ run: # output configuration options output: - # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + # colored-line-number|line-number|json|tab|checkstyle|code-climate, + # default is "colored-line-number" format: colored-line-number # print lines of code with issue, default is true @@ -45,17 +46,19 @@ output: # all available settings of specific linters linters-settings: errcheck: - # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # report about not checking of errors in type assetions: + # `a := b.(MyStruct)`; # default is false: such cases aren't reported by default. check-type-assertions: true - # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # report about assignment of errors to blank identifier: + # `num, _ := strconv.Atoi(numStr)`; # default is false: such cases aren't reported by default. check-blank: true # path to a file containing a list of functions to exclude from checking # see https://github.com/kisielk/errcheck#excluding-functions for details - #exclude: /path/to/file.txt + # exclude: /path/to/file.txt govet: # report about shadowed variables check-shadowing: true @@ -91,7 +94,8 @@ linters-settings: misspell: # Correct spellings using locale preferences for US or UK. # Default is to use a neutral variety of English. - # Setting locale to US will correct the British spelling of 'colour' to 'color'. + # Setting locale to US will correct the British spelling of 'colour' to + # 'color'. locale: US ignore-words: - someword @@ -104,19 +108,26 @@ linters-settings: # tab width in spaces. Default to 1. tab-width: 1 unused: - # treat code as a program (not a library) and report unused exported identifiers; default is false. - # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: - # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # treat code as a program (not a library) and report unused exported + # identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of + # false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. + # All text editor integrations # with golangci-lint call it on a directory with the changed file. check-exported: false unparam: - # Inspect exported functions, default is false. Set to true if no external program/library imports your code. - # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: - # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # Inspect exported functions, default is false. Set to true if no external + # program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of + # false-positives in text editors: + # if it's called for subdir of a project it can't find external + # interfaces. All text editor integrations # with golangci-lint call it on a directory with the changed file. check-exported: false nakedret: - # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + # make an issue if func has more lines of code than this setting and + # it has naked returns; default is 30 max-func-lines: 30 linters: From 0fd091fa7f1c34ff05af053b1ed83466a939f8f5 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 4 Mar 2019 19:02:10 +0530 Subject: [PATCH 4/4] skip errcheck Signed-off-by: Madhu Rajanna --- pkg/rbd/controllerserver.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/rbd/controllerserver.go b/pkg/rbd/controllerserver.go index 598451f9e..c4808e72d 100644 --- a/pkg/rbd/controllerserver.go +++ b/pkg/rbd/controllerserver.go @@ -24,7 +24,7 @@ import ( "strings" "syscall" - "github.com/ceph/ceph-csi/pkg/csi-common" + csicommon "github.com/ceph/ceph-csi/pkg/csi-common" "github.com/ceph/ceph-csi/pkg/util" "github.com/container-storage-interface/spec/lib/go/csi" @@ -201,7 +201,8 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol func (cs *ControllerServer) checkRBDStatus(rbdVol *rbdVolume, req *csi.CreateVolumeRequest, volSizeGB int) error { var err error // Check if there is already RBD image with requested name - found, _, _ := rbdStatus(rbdVol, rbdVol.UserID, req.GetSecrets()) // #nosec + //nolint + found, _, _ := rbdStatus(rbdVol, rbdVol.UserID, req.GetSecrets()) if !found { // if VolumeContentSource is not nil, this request is for snapshot if req.VolumeContentSource != nil {