k8snetworkplumbingwg / ovs-cni

Open vSwitch CNI plugin
Apache License 2.0
220 stars 70 forks source link

Golangci lint #257

Closed phoracek closed 1 year ago

phoracek commented 1 year ago

What this PR does / why we need it:

The existing linter is not catching issues with unhandled errors etc. Golang-lint will check that and more, and will help us keep the codebase a little safer (and maybe even little prettier).

An example of its output:

tests/cmd/run.go:32:27: Error return value of `ginkgo.GinkgoWriter.Write` is not checked (errcheck)
        ginkgo.GinkgoWriter.Write([]byte(command + " " + strings.Join(arguments, " ") + "\n"))
                                 ^
tests/cmd/run.go:37:27: Error return value of `ginkgo.GinkgoWriter.Write` is not checked (errcheck)
        ginkgo.GinkgoWriter.Write([]byte(fmt.Sprintf("stdout: %.500s...\n, stderr %s\n", stdout.String(), stderr.String())))
                                 ^
pkg/plugin/plugin.go:323:17: Error return value of `ipam.ExecDel` is not checked (errcheck)
                                ipam.ExecDel(netconf.IPAM.Type, args.StdinData)
                                            ^
pkg/plugin/plugin.go:473:20: Error return value of `utils.CleanCache` is not checked (errcheck)
                        utils.CleanCache(cRef)
                                        ^
pkg/plugin/plugin.go:551:17: Error return value of `sriov.ResetVF` is not checked (errcheck)
                        sriov.ResetVF(args, cache.Netconf.DeviceID, cache.OrigIfName)
                                     ^
pkg/plugin/plugin.go:562:21: Error return value of `ip.DelLinkByName` is not checked (errcheck)
                                ip.DelLinkByName(portName)
                                                ^
pkg/plugin/plugin_test.go:102:68: Error return value of `(*os/exec.Cmd).Run` is not checked (errcheck)
        exec.Command("ovs-vsctl", "del-br", "--if-exists", bridgeName).Run()
                                                                          ^
pkg/plugin/plugin_test.go:558:25: Error return value of `testutils.UnmountNS` is not checked (errcheck)
                                        testutils.UnmountNS(targetNs)
                                                           ^
pkg/mirror-producer/producer.go:149:20: Error return value of `utils.CleanCache` is not checked (errcheck)
                        utils.CleanCache(cRef + "_prod")
                                        ^
pkg/mirror-producer/producer_test.go:54:68: Error return value of `(*os/exec.Cmd).Run` is not checked (errcheck)
        exec.Command("ovs-vsctl", "del-br", "--if-exists", bridgeName).Run()
                                                                          ^
pkg/mirror-producer/producer_test.go:597:26: Error return value is not checked (errcheck)
                                AddOutputPortToMirror(portUUID, mirrors[0].Name)
                                                     ^
pkg/mirror-consumer/consumer.go:157:20: Error return value of `utils.CleanCache` is not checked (errcheck)
                        utils.CleanCache(cRef + "_cons")
                                        ^
pkg/mirror-consumer/consumer_test.go:54:68: Error return value of `(*os/exec.Cmd).Run` is not checked (errcheck)
        exec.Command("ovs-vsctl", "del-br", "--if-exists", bridgeName).Run()
                                                                          ^
pkg/mirror-consumer/consumer_test.go:402:26: Error return value is not checked (errcheck)
                                AddSelectPortToMirror(portUUID, mirrors[0].Name, true, true)
                                                     ^
pkg/mirror-consumer/consumer_test.go:469:26: Error return value is not checked (errcheck)
                                AddSelectPortToMirror(portUUID, mirrors[0].Name, true, false)
                                                     ^
tests/marker_test.go:37:24: Error return value of `node.RunAtNode` is not checked (errcheck)
                        defer node.RunAtNode("node01", "sudo ovs-vsctl --if-exists del-br br-test")
                                            ^
tests/tests_suite_test.go:46:10: Error return value of `os.Chdir` is not checked (errcheck)
        os.Chdir("../")
                ^
pkg/plugin/plugin.go:50:7: const `macSetupRetries` is unused (unused)
const macSetupRetries = 2
      ^
tests/marker_test.go:47:5: S1008: should use 'return capacityInt == int64(1000)' instead of 'if capacityInt != int64(1000) { return false }; return true' (gosimple)
                                if capacityInt != int64(1000) {
                                ^
pkg/plugin/plugin_test.go:814:2: S1017: should replace this if statement with an unconditional strings.TrimSuffix (gosimple)
        if strings.HasSuffix(extraArgs, ",") {
        ^
cmd/marker/main.go:98:26: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
                shouldReconcileNode := time.Now().Sub(markerCache.LastRefreshTime()) >= jitteredReconcileInterval
                                       ^
tests/cluster/cluster.go:121:8: S1002: should omit comparison to bool constant, can be simplified to `!containerStatus.Ready` (gosimple)
                                if containerStatus.Ready != true {
                                   ^
pkg/plugin/plugin.go:205:7: ineffectual assignment to id (ineffassign)
                var id uint = 0
                    ^
pkg/plugin/plugin.go:187:7: SA4003: no value of type `uint` is less than `0` (staticcheck)
                        if minID < 0 || minID > 4096 {
                           ^
pkg/plugin/plugin.go:193:7: SA4003: no value of type `uint` is less than `0` (staticcheck)
                        if maxID < 0 || maxID > 4096 {
                           ^
pkg/plugin/plugin.go:208:7: SA4003: no value of type `uint` is less than `0` (staticcheck)
                        if id < 0 || minID > 4096 {
                           ^

Release note:

NONE
phoracek commented 1 year ago

/hold

phoracek commented 1 year ago

/hold cancel

phoracek commented 1 year ago

/retest

qinqon commented 1 year ago

/lgtm /approve

kubevirt-bot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: phoracek, qinqon

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: - ~~[OWNERS](https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/OWNERS)~~ [phoracek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment