Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix gate lint issue #980

Merged
merged 1 commit into from
Aug 30, 2021
Merged

Conversation

jichenjc
Copy link
Contributor

@jichenjc jichenjc commented Aug 26, 2021

What this PR does / why we need it:
due to go 1.17 usage, have following error in test

test/e2e/shared/cluster.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/common.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/context.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/defaults.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/exec.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/openstack.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/suite.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/exec_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/openstack_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/conformance/conformance_suite_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/conformance/conformance_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/e2e/e2e_suite_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/e2e/e2e_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • if necessary:
    • includes documentation
    • adds unit tests

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 26, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jichenjc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 26, 2021
@tobiasgiese
Copy link
Member

A local make lint and make test (i.e., ./scripts/ci-test.sh) is working properly without any issues 🤔

@jichenjc
Copy link
Contributor Author

A local make lint and make test (i.e., ./scripts/ci-test.sh) is working properly without any issue

weird, but will check how to fix the issue based on current CI running env

@jichenjc jichenjc force-pushed the test_gate branch 2 times, most recently from 55ef14f to 0fbad12 Compare August 26, 2021 09:29
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 26, 2021
@jichenjc jichenjc changed the title test gate fix gate lint issue Aug 26, 2021
@jichenjc jichenjc force-pushed the test_gate branch 2 times, most recently from 02aa776 to 7955410 Compare August 26, 2021 11:08
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 26, 2021
@seanschneeweiss
Copy link
Contributor

/uncc @seanschneeweiss

@k8s-ci-robot k8s-ci-robot removed the request for review from seanschneeweiss August 26, 2021 13:14
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 30, 2021
@jichenjc
Copy link
Contributor Author

@tobiasgiese please check above , it should be fine now :)

@tobiasgiese
Copy link
Member

tobiasgiese commented Aug 30, 2021

@tobiasgiese please check above , it should be fine now :)

@jichenjc were you able to reproduce it. Because, even with a local go 1.17 env I'm not able to catch these errors with a fresh master branch.

$ go version
go version go1.17 linux/amd64
$ make lint 
hack/tools/bin/golangci-lint run -v --fast=false
INFO [config_reader] Config search paths: [./ /home/tobias/projects/github.com/Daimler/cluster-api-provider-openstack /home/tobias/projects/github.com/Daimler /home/tobias/projects/github.com /home/tobias/projects /home/tobias /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 50 linters: [asciicheck bodyclose cyclop deadcode depguard dogsled durationcheck errcheck exportloopref forbidigo gci goconst gocritic gocyclo godot gofmt gofumpt goheader goimports gomodguard goprintffuncname gosec gosimple govet ifshort importas ineffassign makezero misspell nakedret nestif nilerr noctx nolintlint prealloc predeclared revive rowserrcheck sqlclosecheck staticcheck structcheck stylecheck thelper typecheck unconvert unparam unused varcheck wastedassign whitespace] 
INFO [loader] Using build tags: [e2e]             
INFO [loader] Go packages loading at mode 575 (exports_file|imports|types_sizes|compiled_files|deps|files|name) took 613.131259ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 6.287711ms 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 369, after processing: 0 
INFO [runner] Processors filtering stat (out/in): path_prettifier: 369/369, skip_dirs: 226/226, autogenerated_exclude: 142/226, exclude-rules: 5/142, cgo: 369/369, identifier_marker: 142/142, filename_unadjuster: 369/369, skip_files: 226/369, exclude: 142/142, nolint: 0/5 
INFO [runner] processing took 10.02371ms with stages: exclude-rules: 5.275212ms, identifier_marker: 2.297505ms, path_prettifier: 712.792µs, autogenerated_exclude: 600.833µs, nolint: 541.114µs, skip_files: 407.046µs, skip_dirs: 143.738µs, cgo: 31.18µs, filename_unadjuster: 12.23µs, max_same_issues: 360ns, uniq_by_line: 310ns, source_code: 220ns, exclude: 220ns, diff: 190ns, severity-rules: 180ns, max_from_linter: 150ns, path_shortener: 130ns, max_per_file_from_linter: 110ns, sort_results: 110ns, path_prefixer: 80ns 
INFO [runner] linters took 151.328031ms with stages: goanalysis_metalinter: 141.244071ms 
INFO File cache stats: 35 entries of total size 244.0KiB 
INFO Memory: 9 samples, avg is 87.4MB, max is 139.8MB 
INFO Execution took 774.320901ms                  

I think we should find the root cause and also a way to reproduce these linting errors. Otherwise, we're not able to fix these linting errors locally.

@tobiasgiese
Copy link
Member

tobiasgiese commented Aug 30, 2021

@jichenjc my fault, it's make verify to validate these errors. I was confused because it's directly after the linting target.

Edit: still facing the issues. But I think it's a problem of my env, as Prow is the source of truth

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2021
@jichenjc
Copy link
Contributor Author

@tobiasgiese I can reproduce it my local env...

root@jitest19:~/go/src/cluster-api-provider-openstack# git log
commit 881c24b3c15eb3aa1a73dc794049d31b8254c064 (HEAD -> master, upstream/master)
Merge: 79390910 fec4f2aa


root@jitest19:~/go/src/cluster-api-provider-openstack# hack/tools/bin/golangci-lint run -v --fast=false
INFO [config_reader] Config search paths: [./ /root/go/src/cluster-api-provider-openstack /root/go/src /root/go /root /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 50 linters: [asciicheck bodyclose cyclop deadcode depguard dogsled durationcheck errcheck exportloopref forbidigo gci goconst gocritic gocyclo godot gofmt gofumpt goheader goimports gomodguard goprintffuncname gosec gosimple govet ifshort importas ineffassign makezero misspell nakedret nestif nilerr noctx nolintlint prealloc predeclared revive rowserrcheck sqlclosecheck staticcheck structcheck stylecheck thelper typecheck unconvert unparam unused varcheck wastedassign whitespace]
INFO [loader] Using build tags: [e2e]
INFO [loader] Go packages loading at mode 575 (exports_file|name|compiled_files|deps|files|imports|types_sizes) took 981.404503ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 15.56684ms
INFO [linters context/goanalysis] analyzers took 31.668295583s with top 10 stages: the_only_name: 4.11784014s, wastedassign: 2.293128331s, buildir: 2.28143604s, buildssa: 1.939222566s, goimports: 1.633351167s, gosec: 1.410633679s, forbidigo: 1.307128232s, gofmt: 1.059273952s, gofumpt: 836.935311ms, directives: 678.9931ms
INFO [runner] Issues before processing: 58689, after processing: 13
INFO [runner] Processors filtering stat (out/in): max_per_file_from_linter: 13/13, cgo: 58689/58689, filename_unadjuster: 58689/58689, path_prettifier: 58689/58689, skip_files: 272/58689, skip_dirs: 272/272, max_same_issues: 13/13, source_code: 13/13, severity-rules: 13/13, sort_results: 13/13, max_from_linter: 13/13, path_prefixer: 13/13, identifier_marker: 188/188, exclude: 188/188, exclude-rules: 44/188, uniq_by_line: 13/39, diff: 13/13, autogenerated_exclude: 188/272, nolint: 39/44, path_shortener: 13/13
INFO [runner] processing took 95.618895ms with stages: path_prettifier: 35.99239ms, skip_files: 32.952615ms, exclude-rules: 7.691001ms, cgo: 5.501995ms, nolint: 4.409032ms, identifier_marker: 3.979922ms, filename_unadjuster: 3.615595ms, autogenerated_exclude: 1.103726ms, skip_dirs: 205.872µs, source_code: 132.718µs, max_per_file_from_linter: 19.726µs, uniq_by_line: 7.929µs, path_shortener: 3.241µs, max_same_issues: 1.425µs, diff: 401ns, max_from_linter: 367ns, exclude: 337ns, severity-rules: 302ns, sort_results: 173ns, path_prefixer: 128ns
INFO [runner] linters took 1.842555171s with stages: goanalysis_metalinter: 1.746533699s
test/e2e/suites/conformance/conformance_suite_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/conformance/conformance_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/e2e/e2e_suite_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/suites/e2e/e2e_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/cluster.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/common.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/context.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/defaults.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/exec.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/openstack.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/suite.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/exec_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
test/e2e/shared/openstack_test.go:1: File is not `gofmt`-ed with `-s` (gofmt)
// +build e2e
INFO File cache stats: 111 entries of total size 776.1KiB
INFO Memory: 30 samples, avg is 336.2MB, max is 829.3MB
INFO Execution took 2.849555977s

@jichenjc
Copy link
Contributor Author

root@jitest19:~/go/src/cluster-api-provider-openstack# go version
go version go1.17 linux/amd64

@jichenjc
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2021
@k8s-ci-robot k8s-ci-robot merged commit 4e178f0 into kubernetes-sigs:master Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants