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

Fixed static check failures for pkg/volume/, pkg/kubelet, pkg/util and cmd/kubelet #81688

Open
wants to merge 5 commits into
base: master
from

Conversation

@hpandeycodeit
Copy link
Member

commented Aug 20, 2019

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Fixing Static Check Failures for the following packages

cmd/kubelet/app
pkg/util/rlimit/
pkg/kubelet/dockershim
pkg/volume/util/operationexecutor
pkg/volume/util/fsquota
pkg/kubelet/dockershim/libdocker

Which issue(s) this PR fixes:

Ref: #81657

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE
@xichengliudui

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2019

It should be ref #81657 , not Fixes #81657, which will lead to the issue closing

@@ -45,8 +45,6 @@ pkg/kubelet/cm/devicemanager
pkg/kubelet/config
pkg/kubelet/container/testing
pkg/kubelet/dockershim/libdocker

This comment has been minimized.

Copy link
@RainbowMango

RainbowMango Aug 21, 2019

Member

Why not fix pkg/kubelet/dockershim/libdocker as well?

pkg/volume/util/fsquota
pkg/volume/util/fsquota/common
pkg/volume/util/nsenter
pkg/volume/util/operationexecutor

This comment has been minimized.

Copy link
@RainbowMango

RainbowMango Aug 21, 2019

Member

Why not fix pkg/volume/util/operationexecutor as well.

@@ -117,12 +115,8 @@ pkg/volume/quobyte
pkg/volume/rbd
pkg/volume/scaleio
pkg/volume/storageos
pkg/volume/util
pkg/volume/util/fsquota

This comment has been minimized.

Copy link
@RainbowMango

RainbowMango Aug 21, 2019

Member

Why not fix pkg/volume/util/fsquota as well?

This comment has been minimized.

Copy link
@hpandeycodeit

hpandeycodeit Aug 21, 2019

Author Member

Sure. I will fix the following as well :

pkg/kubelet/dockershim/libdocker
pkg/volume/util/operationexecutor
pkg/volume/util/fsquota
Copy link
Contributor

left a comment

Thanks for your pr!

Would you mind updating the title? It doesn't feel like it fully describes all the different aspects of this pr (i.e. you fixed static check failures for more than just the kubelet).

Also, I think the kubelet specific fixes are also in #81206 (cc @tallclair), so we just want to make sure there isn't a conflict when it comes time to merge.

@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

/retest

@hpandeycodeit hpandeycodeit changed the title fixed static check failures for pkg/kubelet Fixed static check failures for pkg/volume/util:operationexecutor, fsquota , pkg/kubelet/dockershim , pkg/util/rlimit/ and cmd/kubelet/appand Aug 21, 2019
@hpandeycodeit hpandeycodeit changed the title Fixed static check failures for pkg/volume/util:operationexecutor, fsquota , pkg/kubelet/dockershim , pkg/util/rlimit/ and cmd/kubelet/appand Fixed static check failures for pkg/volume/util:operationexecutor, fsquota , pkg/kubelet/dockershim , pkg/util/rlimit/ and cmd/kubelet/app Aug 21, 2019
@hpandeycodeit hpandeycodeit changed the title Fixed static check failures for pkg/volume/util:operationexecutor, fsquota , pkg/kubelet/dockershim , pkg/util/rlimit/ and cmd/kubelet/app Fixed static check failures for pkg/volume/, pkg/kubelet, pkg/util and cmd/kubelet Aug 21, 2019
@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch 2 times, most recently from b326e3c to 6c43987 Aug 21, 2019
@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2019

The static check errors reported here are not reported in my local. Am I missing something?
@SataQiu

Errors from staticcheck:
pkg/kubelet/dockershim/network/cni/cni_test.go:75:5: this value of err is never used (SA4006)
pkg/kubelet/dockershim/network/cni/cni_test.go:331:10: this value of err is never used (SA4006)
pkg/kubelet/dockershim/network/kubenet/kubenet_linux.go:69:2: const defaultIPAMDir is unused (U1000)
pkg/volume/util/atomic_writer_test.go:756:49: argument err is overwritten before first use (SA4009)
pkg/volume/util/fsquota/common/quota_linux_common.go:37:2: const acct is unused (U1000)
pkg/volume/util/fsquota/common/quota_linux_common.go:38:2: const enforcing is unused (U1000)
pkg/volume/util/fsquota/project.go:100:5: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:100:21: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/project.go:103:5: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:103:21: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/project.go:106:4: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:106:20: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/project.go:168:31: identical expressions on the left and right side of the '==' operator (SA4000)
pkg/volume/util/fsquota/quota_linux.go:227:18: this value of err is never used (SA4006)
pkg/volume/util/fsquota/quota_linux.go:300:50: argument poduid is overwritten before first use (SA4009)
pkg/volume/util/fsquota/quota_linux.go:412:10: this value of err is never used (SA4006)
pkg/volume/util/fsquota/quota_linux_test.go:46:7: const dummyMountDataPquota is unused (U1000)
pkg/volume/util/fsquota/quota_linux_test.go:52:7: const dummyMountDataNoPquota is unused (U1000)
pkg/volume/util/fsquota/quota_linux_test.go:59:7: const dummyMountTest is unused (U1000)
pkg/volume/util/fsquota/quota_linux_test.go:577:16: this value of err is never used (SA4006)
pkg/volume/util/nsenter/nsenter_mount.go:112:9: empty branch (SA9003)
pkg/volume/util/subpath/subpath_linux.go:234:81: argument err is overwritten before first use (SA4009)

My Mac:

./hack/verify-staticcheck.sh
installing staticcheck from vendor
Congratulations!  All Go source files have been checked.
@SataQiu

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

Thanks for your contribution @hpandeycodeit
First, you need change your PR description : Fixes #81657 -> Ref: #81657
Second, I think the reason the error has not been detected is that you have not installed staticcheck correctly. Try to install it by:

$ go install honnef.co/go/tools/cmd/staticcheck
$ export PATH=${GOPATH}/bin:${PATH}
$ ./hack/verify-staticcheck.sh

/approve
for hack

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hpandeycodeit, SataQiu
To complete the pull request process, please assign dchen1107
You can assign the PR to them by writing /assign @dchen1107 in a comment when ready.

The full list of commands accepted by this bot can be found 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


dockerNetNSFmt = "/proc/%v/ns/net"

)

This comment has been minimized.

Copy link
@tallclair

tallclair Aug 23, 2019

Member

nit: too much whitespace. Prefer:

var defaultSeccompOpt = []dockerOpt{{"seccomp", "unconfined", ""}}

const dockerNetNSFmt = "/proc/%v/ns/net"
@@ -44,6 +44,6 @@ type Interface interface {
ClearQuota(m mount.Interface, path string) error
}

func enabledQuotasForMonitoring() bool {
func EnabledQuotasForMonitoring() bool {

This comment has been minimized.

Copy link
@tallclair

tallclair Aug 23, 2019

Member

Why did you export this?

This comment has been minimized.

Copy link
@hpandeycodeit

hpandeycodeit Aug 23, 2019

Author Member

staticcheck reported this function never used. However, /pkg/volume/util/fsquota/quota_linux.go is calling this function (same package scope)

This comment has been minimized.

Copy link
@mkumatag

mkumatag Aug 31, 2019

Member

Even I have a same question if not exported outside of the package scope then no need to export(CapsCase)

@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from 55a3473 to 39bd885 Sep 3, 2019
@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2019

/retest

@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from 297bb9c to 274c6d4 Sep 10, 2019
@k8s-ci-robot k8s-ci-robot added size/M and removed size/L labels Sep 10, 2019
@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch 4 times, most recently from 5f27810 to 32eb803 Sep 10, 2019
@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from 32eb803 to 75c8a96 Sep 10, 2019
@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

/retest

@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from cab29bc to 8324591 Sep 10, 2019
@RainbowMango

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

@hpandeycodeit

I guess you should rebase master then fix staticcheck issues.

Errors from staticcheck:
pkg/volume/util/fsquota/project.go:101:5: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:101:21: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/project.go:103:5: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:103:21: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/project.go:106:4: this value of err is never used (SA4006)
pkg/volume/util/fsquota/project.go:106:20: Errorf is a pure function but its return value is ignored (SA4017)
pkg/volume/util/fsquota/quota_linux.go:303:50: argument poduid is overwritten before first use (SA4009)
@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from 8324591 to 2c41991 Sep 11, 2019
@hpandeycodeit hpandeycodeit force-pushed the hpandeycodeit:static_check branch from 2c41991 to abc60ba Sep 11, 2019
@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

/test pull-kubernetes-e2e-gce

@hpandeycodeit

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

/test pull-kubernetes-e2e-gce-storage-slow
/test pull-kubernetes-e2e-gce-csi-serial
/test pull-kubernetes-node-e2e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.