Add golangci-lint support #895

mrueg · 2020-05-05T13:19:16Z

This adds support for golangci-lint to improve code quality https://github.com/golangci/golangci-lint

mrueg · 2020-05-05T21:17:32Z

I'll try to get the findings fixed this weekend.

mrueg

@aauren @murali-reddy I annotated the PR to help with the review.

mrueg · 2020-05-14T17:41:43Z

cmd/kube-router/kube-router_test.go

-		io.Copy(stderrBuf, stderrR)
+		_, err := io.Copy(stderrBuf, stderrR)
+		if err != nil {
+			panic(err)


Not sure if panic is the best way to respond here, open for suggestions.

pkg/cmd/kube-router.go

pkg/controllers/proxy/network_services_controller.go

pkg/controllers/proxy/service_endpoints_sync.go

pkg/controllers/routing/network_routes_controller.go

pkg/healthcheck/health_controller.go

pkg/metrics/metrics_controller.go

murali-reddy · 2020-05-28T11:39:38Z

Huh. That was long list of changes. Changes look good. I hope there wont be any regression, if at all they should be more like exposing flase postive case that was ignored earlier.

I will skim through once again non test changes. Meanwhile any extra eyes for review would be welcome.

aauren

Wow! This was a big effort. It took me at least an hour to review, so I can't imagine how much time it took to make. Thanks @mrueg !

cmd/kube-router/kube-router_test.go

pkg/cmd/kube-router.go

aauren · 2020-05-29T21:36:42Z

pkg/controllers/netpol/network_policy_controller.go

 	// find iptables chains and ipsets that are no longer used by comparing current to the active maps we were passed
 	chains, err := iptablesCmdHandler.ListChains("filter")
+	if err != nil {
+		return fmt.Errorf("Unable to list chains: %s", err)


@murali-reddy Is there any reason why we wouldn't want to return for all of the err conditions above too? Right now we only fatally log them, but if we can't get an valid iptables or ipset handle, I can't imagine we're going to be able to do much in this method...

The goal of this PR is to make the linter pass, I guess an issue to improve and unify error handling would be good.

Is there any reason why we wouldn't want to return for all of the err conditions above too?

@aauren Yes. I belive intent is its fatal condition if we can not get a valid handle. logging FATAL message terminates the program.

pkg/controllers/proxy/network_services_controller.go

aauren · 2020-05-29T21:54:47Z

pkg/controllers/proxy/network_services_controller.go

+			if err != nil {
+				return errors.New("Failed to set hostNetworkNamespace handle due to " + err.Error())
+			}
 			activeNetworkNamespaceHandle, err = netns.Get()


@murali-reddy I'm thinking that this is another problem for us. We're in this block because we got an error, but here we're going to lose that error if the call to netns.Get() is successful, before we can return it 3 lines below this one.

aauren · 2020-05-29T22:10:07Z

pkg/controllers/proxy/service_endpoints_sync.go

-
+				if err != nil {
+					glog.Errorf("Failed to delete external ip adress from dummyVipInterface due to %s", err)
+					continue


I think continuing here is correct. That allow any other VIPs that might be able to be setup to continue to be setup rather than just halting where it is.

pkg/controllers/proxy/service_endpoints_sync.go

pkg/controllers/routing/network_routes_controller.go

pkg/controllers/proxy/network_service_graceful.go:21:6: `gracefulQueueItem` is unused (deadcode) type gracefulQueueItem struct { ^ pkg/controllers/proxy/network_service_graceful.go:22:2: `added` is unused (structcheck) added time.Time ^ pkg/controllers/proxy/network_service_graceful.go:23:2: `service` is unused (structcheck) service *ipvs.Service ^

pkg/metrics/metrics_controller.go:150:2: `mu` is unused (structcheck) mu sync.Mutex ^ pkg/metrics/metrics_controller.go:151:2: `nodeIP` is unused (structcheck) nodeIP net.IP ^

pkg/controllers/routing/ecmp_vip.go:208:4: S1023: redundant `return` statement (gosimple) return ^

pkg/controllers/routing/bgp_peers.go:331:4: S1023: redundant `return` statement (gosimple) return ^

pkg/utils/ipset.go:243:23: Error return value of `entry.Set.Parent.Save` is not checked (errcheck) entry.Set.Parent.Save() ^

cmd/kube-router/kube-router_test.go:21:10: Error return value of `io.Copy` is not checked (errcheck) io.Copy(stderrBuf, stderrR) ^ cmd/kube-router/kube-router_test.go:40:17: Error return value of `docBuf.ReadFrom` is not checked (errcheck) docBuf.ReadFrom(docF) ^

cmd/kube-router/kube-router.go:31:24: Error return value of `flag.CommandLine.Parse` is not checked (errcheck) flag.CommandLine.Parse([]string{}) ^ cmd/kube-router/kube-router.go:33:10: Error return value of `flag.Set` is not checked (errcheck) flag.Set("logtostderr", "true") ^ cmd/kube-router/kube-router.go:34:10: Error return value of `flag.Set` is not checked (errcheck) flag.Set("v", config.VLevel) ^ cmd/kube-router/kube-router.go:62:27: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error()) ^

pkg/utils/node.go:19:16: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err := clientset.Core().Nodes().Get(nodeName, metav1.GetOptions{}) ^ pkg/utils/node.go:27:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err := clientset.Core().Nodes().Get(hostName, metav1.GetOptions{}) ^ pkg/utils/node.go:34:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err = clientset.Core().Nodes().Get(hostnameOverride, metav1.GetOptions{}) ^

pkg/controllers/routing/bgp_policies.go:80:3: S1011: should replace loop with `externalBgpPeers = append(externalBgpPeers, nrc.nodePeerRouters...)` (gosimple) for _, peer := range nrc.nodePeerRouters { ^ pkg/controllers/routing/bgp_policies.go:23:20: ineffectual assignment to `err` (ineffassign) podCidrPrefixSet, err := table.NewPrefixSet(config.PrefixSet{ ^ pkg/controllers/routing/bgp_policies.go:42:22: ineffectual assignment to `err` (ineffassign) clusterIPPrefixSet, err := table.NewPrefixSet(config.PrefixSet{ ^ pkg/controllers/routing/bgp_policies.go:33:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(podCidrPrefixSet) ^ pkg/controllers/routing/bgp_policies.go:48:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet) ^ pkg/controllers/routing/bgp_policies.go:69:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(iBGPPeerNS) ^ pkg/controllers/routing/bgp_policies.go:108:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(ns) ^ pkg/controllers/routing/bgp_policies.go:120:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(ns) ^ ^

pkg/controllers/netpol/network_policy_controller.go:35:2: `networkPolicyAnnotation` is unused (deadcode) networkPolicyAnnotation = "net.beta.kubernetes.io/network-policy" ^ pkg/controllers/netpol/network_policy_controller.go:1047:4: SA9003: empty branch (staticcheck) if err != nil { ^ pkg/controllers/netpol/network_policy_controller.go:969:10: SA4006: this value of `err` is never used (staticcheck) chains, err := iptablesCmdHandler.ListChains("filter") ^ pkg/controllers/netpol/network_policy_controller.go:1568:4: SA4006: this value of `err` is never used (staticcheck) err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo)) ^ pkg/controllers/netpol/network_policy_controller.go:1584:4: SA4006: this value of `err` is never used (staticcheck) err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo)) ^

pkg/controllers/routing/aws.go:31:8: SA4006: this value of `err` is never used (staticcheck) URL, err := url.Parse(providerID) ^

pkg/controllers/proxy/network_services_controller_test.go:80:6: func `logf` is unused (unused)

pkg/controllers/proxy/network_services_controller.go:66:2: `h` is unused (deadcode) h *ipvs.Handle ^ pkg/controllers/proxy/network_services_controller.go:879:23: SA1019: client.NewEnvClient is deprecated: use NewClientWithOpts(FromEnv) (staticcheck) dockerClient, err := client.NewEnvClient() ^ pkg/controllers/proxy/network_services_controller.go:944:5: unreachable: unreachable code (govet) glog.V(3).Infof("Waiting for tunnel interface %s to come up in the pod, retrying", KUBE_TUNNEL_IF) ^ pkg/controllers/proxy/network_services_controller.go:1289:5: S1002: should omit comparison to bool constant, can be simplified to `!hasHairpinChain` (gosimple) if hasHairpinChain != true { ^ pkg/controllers/proxy/network_services_controller.go:1237:43: S1019: should use make(map[string][]string) instead (gosimple) rulesNeeded := make(map[string][]string, 0) ^ pkg/controllers/proxy/network_services_controller.go:1111:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:1114:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:1117:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:445:21: Error return value of `nsc.publishMetrics` is not checked (errcheck) nsc.publishMetrics(nsc.serviceMap) ^ pkg/controllers/proxy/network_services_controller.go:1609:9: Error return value of `h.Write` is not checked (errcheck) h.Write([]byte(ip + "-" + protocol + "-" + port)) ^ pkg/controllers/proxy/network_services_controller.go:912:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:926:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:950:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:641:9: SA4006: this value of `err` is never used (staticcheck) addrs, err := getAllLocalIPs() ^

pkg/controllers/proxy/service_endpoints_sync.go:460:2: ineffectual assignment to `ipvsSvcs` (ineffassign) ipvsSvcs, err := nsc.ln.ipvsGetServices() ^ pkg/controllers/proxy/service_endpoints_sync.go:311:5: SA4006: this value of `err` is never used (staticcheck) err = nsc.ln.ipAddrDel(dummyVipInterface, externalIP) ^

pkg/cmd/kube-router.go:214:26: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(os.Stderr, output) ^ pkg/cmd/kube-router.go:184:15: SA1017: the channel used with signal.Notify should be buffered (staticcheck) signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM) ^ pkg/cmd/kube-router.go:94:17: Error return value of `hc.RunServer` is not checked (errcheck) go hc.RunServer(stopCh, &wg) ^ pkg/cmd/kube-router.go:112:16: Error return value of `hc.RunCheck` is not checked (errcheck) go hc.RunCheck(healthChan, stopCh, &wg) ^ pkg/cmd/kube-router.go:121:12: Error return value of `mc.Run` is not checked (errcheck) go mc.Run(healthChan, stopCh, &wg) ^

pkg/controllers/routing/network_routes_controller_test.go:1113:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller_test.go:1314:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller_test.go:2327:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^

pkg/controllers/routing/network_routes_controller.go:340:2: S1000: should use for range instead of for { select {} } (gosimple) for { ^ pkg/controllers/routing/network_routes_controller.go:757:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:770:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:782:23: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:717:12: Error return value of `g.Serve` is not checked (errcheck) go g.Serve()

pkg/healthcheck/health_controller.go:54:10: Error return value of `w.Write` is not checked (errcheck) w.Write([]byte("OK\n")) ^ pkg/healthcheck/health_controller.go:68:10: Error return value of `w.Write` is not checked (errcheck) w.Write([]byte("Unhealthy")) ^ pkg/healthcheck/health_controller.go:159:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple) select { ^

Default is 1m, increase to 5m otherwise travis might fail

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>

aauren · 2020-06-01T20:26:08Z

@murali-reddy Technically it's been reviewed twice, so I think this guy is good to go. If you don't have anything else, are you ok if we merge it?

murali-reddy · 2020-06-03T17:28:00Z

@aauren Please go ahead with the merge. If you are satisfied with the changes.

* Makefile: Add lint using golangci-lint * build/travis-test.sh: Run lint step * metrics_controller: Lint pkg/metrics/metrics_controller.go:150:2: `mu` is unused (structcheck) mu sync.Mutex ^ pkg/metrics/metrics_controller.go:151:2: `nodeIP` is unused (structcheck) nodeIP net.IP ^ * network_service_graceful: Lint pkg/controllers/proxy/network_service_graceful.go:21:6: `gracefulQueueItem` is unused (deadcode) type gracefulQueueItem struct { ^ pkg/controllers/proxy/network_service_graceful.go:22:2: `added` is unused (structcheck) added time.Time ^ pkg/controllers/proxy/network_service_graceful.go:23:2: `service` is unused (structcheck) service *ipvs.Service ^ * network_services_controller_test: Lint pkg/controllers/proxy/network_services_controller_test.go:80:6: func `logf` is unused (unused) * ecmp_vip: Lint pkg/controllers/routing/ecmp_vip.go:208:4: S1023: redundant `return` statement (gosimple) return ^ * bgp_peers: Lint pkg/controllers/routing/bgp_peers.go:331:4: S1023: redundant `return` statement (gosimple) return ^ * bgp_policies: Lint pkg/controllers/routing/bgp_policies.go:80:3: S1011: should replace loop with `externalBgpPeers = append(externalBgpPeers, nrc.nodePeerRouters...)` (gosimple) for _, peer := range nrc.nodePeerRouters { ^ pkg/controllers/routing/bgp_policies.go:23:20: ineffectual assignment to `err` (ineffassign) podCidrPrefixSet, err := table.NewPrefixSet(config.PrefixSet{ ^ pkg/controllers/routing/bgp_policies.go:42:22: ineffectual assignment to `err` (ineffassign) clusterIPPrefixSet, err := table.NewPrefixSet(config.PrefixSet{ ^ pkg/controllers/routing/bgp_policies.go:33:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(podCidrPrefixSet) ^ pkg/controllers/routing/bgp_policies.go:48:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet) ^ pkg/controllers/routing/bgp_policies.go:69:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(iBGPPeerNS) ^ pkg/controllers/routing/bgp_policies.go:108:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(ns) ^ pkg/controllers/routing/bgp_policies.go:120:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck) nrc.bgpServer.AddDefinedSet(ns) ^ ^ * network_policy_controller: Lint pkg/controllers/netpol/network_policy_controller.go:35:2: `networkPolicyAnnotation` is unused (deadcode) networkPolicyAnnotation = "net.beta.kubernetes.io/network-policy" ^ pkg/controllers/netpol/network_policy_controller.go:1047:4: SA9003: empty branch (staticcheck) if err != nil { ^ pkg/controllers/netpol/network_policy_controller.go:969:10: SA4006: this value of `err` is never used (staticcheck) chains, err := iptablesCmdHandler.ListChains("filter") ^ pkg/controllers/netpol/network_policy_controller.go:1568:4: SA4006: this value of `err` is never used (staticcheck) err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo)) ^ pkg/controllers/netpol/network_policy_controller.go:1584:4: SA4006: this value of `err` is never used (staticcheck) err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo)) ^ * network_services_controller: Lint pkg/controllers/proxy/network_services_controller.go:66:2: `h` is unused (deadcode) h *ipvs.Handle ^ pkg/controllers/proxy/network_services_controller.go:879:23: SA1019: client.NewEnvClient is deprecated: use NewClientWithOpts(FromEnv) (staticcheck) dockerClient, err := client.NewEnvClient() ^ pkg/controllers/proxy/network_services_controller.go:944:5: unreachable: unreachable code (govet) glog.V(3).Infof("Waiting for tunnel interface %s to come up in the pod, retrying", KUBE_TUNNEL_IF) ^ pkg/controllers/proxy/network_services_controller.go:1289:5: S1002: should omit comparison to bool constant, can be simplified to `!hasHairpinChain` (gosimple) if hasHairpinChain != true { ^ pkg/controllers/proxy/network_services_controller.go:1237:43: S1019: should use make(map[string][]string) instead (gosimple) rulesNeeded := make(map[string][]string, 0) ^ pkg/controllers/proxy/network_services_controller.go:1111:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:1114:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:1117:4: S1023: redundant break statement (gosimple) break ^ pkg/controllers/proxy/network_services_controller.go:445:21: Error return value of `nsc.publishMetrics` is not checked (errcheck) nsc.publishMetrics(nsc.serviceMap) ^ pkg/controllers/proxy/network_services_controller.go:1609:9: Error return value of `h.Write` is not checked (errcheck) h.Write([]byte(ip + "-" + protocol + "-" + port)) ^ pkg/controllers/proxy/network_services_controller.go:912:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:926:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:950:13: Error return value of `netns.Set` is not checked (errcheck) netns.Set(hostNetworkNamespaceHandle) ^ pkg/controllers/proxy/network_services_controller.go:641:9: SA4006: this value of `err` is never used (staticcheck) addrs, err := getAllLocalIPs() ^ * network_routes_controller: Lint pkg/controllers/routing/network_routes_controller.go:340:2: S1000: should use for range instead of for { select {} } (gosimple) for { ^ pkg/controllers/routing/network_routes_controller.go:757:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:770:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:782:23: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck) nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller.go:717:12: Error return value of `g.Serve` is not checked (errcheck) go g.Serve() * ipset: Lint pkg/utils/ipset.go:243:23: Error return value of `entry.Set.Parent.Save` is not checked (errcheck) entry.Set.Parent.Save() ^ * pkg/cmd/kube-router: Lint pkg/cmd/kube-router.go:214:26: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(os.Stderr, output) ^ pkg/cmd/kube-router.go:184:15: SA1017: the channel used with signal.Notify should be buffered (staticcheck) signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM) ^ pkg/cmd/kube-router.go:94:17: Error return value of `hc.RunServer` is not checked (errcheck) go hc.RunServer(stopCh, &wg) ^ pkg/cmd/kube-router.go:112:16: Error return value of `hc.RunCheck` is not checked (errcheck) go hc.RunCheck(healthChan, stopCh, &wg) ^ pkg/cmd/kube-router.go:121:12: Error return value of `mc.Run` is not checked (errcheck) go mc.Run(healthChan, stopCh, &wg) ^ * cmd/kube-router/kube-router: Lint cmd/kube-router/kube-router.go:31:24: Error return value of `flag.CommandLine.Parse` is not checked (errcheck) flag.CommandLine.Parse([]string{}) ^ cmd/kube-router/kube-router.go:33:10: Error return value of `flag.Set` is not checked (errcheck) flag.Set("logtostderr", "true") ^ cmd/kube-router/kube-router.go:34:10: Error return value of `flag.Set` is not checked (errcheck) flag.Set("v", config.VLevel) ^ cmd/kube-router/kube-router.go:62:27: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error()) ^ * kube-router_test: Lint cmd/kube-router/kube-router_test.go:21:10: Error return value of `io.Copy` is not checked (errcheck) io.Copy(stderrBuf, stderrR) ^ cmd/kube-router/kube-router_test.go:40:17: Error return value of `docBuf.ReadFrom` is not checked (errcheck) docBuf.ReadFrom(docF) ^ * service_endpoints_sync: Lint pkg/controllers/proxy/service_endpoints_sync.go:460:2: ineffectual assignment to `ipvsSvcs` (ineffassign) ipvsSvcs, err := nsc.ln.ipvsGetServices() ^ pkg/controllers/proxy/service_endpoints_sync.go:311:5: SA4006: this value of `err` is never used (staticcheck) err = nsc.ln.ipAddrDel(dummyVipInterface, externalIP) ^ * node: Lint pkg/utils/node.go:19:16: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err := clientset.Core().Nodes().Get(nodeName, metav1.GetOptions{}) ^ pkg/utils/node.go:27:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err := clientset.Core().Nodes().Get(hostName, metav1.GetOptions{}) ^ pkg/utils/node.go:34:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible. (staticcheck) node, err = clientset.Core().Nodes().Get(hostnameOverride, metav1.GetOptions{}) ^ * aws: Lint pkg/controllers/routing/aws.go:31:8: SA4006: this value of `err` is never used (staticcheck) URL, err := url.Parse(providerID) ^ * health_controller: Lint pkg/healthcheck/health_controller.go:54:10: Error return value of `w.Write` is not checked (errcheck) w.Write([]byte("OK\n")) ^ pkg/healthcheck/health_controller.go:68:10: Error return value of `w.Write` is not checked (errcheck) w.Write([]byte("Unhealthy")) ^ pkg/healthcheck/health_controller.go:159:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple) select { ^ * network_routes_controller_test: Lint pkg/controllers/routing/network_routes_controller_test.go:1113:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller_test.go:1314:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^ pkg/controllers/routing/network_routes_controller_test.go:2327:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck) defer testcase.nrc.bgpServer.Stop() ^ * .golangci.yml: Increase timeout Default is 1m, increase to 5m otherwise travis might fail * Makefile: Update golangci-lint to 1.27.0 * kube-router_test.go: defer waitgroup Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com> * network_routes_controller: Incorporate review * bgp_policies: Incorporate review * network_routes_controller: Incorporate review * bgp_policies: Log error instead * network_services_controller: Incorporate review Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>

mrueg force-pushed the lint branch 11 times, most recently from 2df8b0a to e79db89 Compare May 10, 2020 13:12

mrueg changed the title ~~[WIP] Add golangci-lint support~~ Add golangci-lint support May 10, 2020

mrueg requested review from aauren and murali-reddy May 10, 2020 13:13

mrueg force-pushed the lint branch 2 times, most recently from 2684c55 to 73c1f22 Compare May 10, 2020 13:21

mrueg commented May 14, 2020

View reviewed changes

mrueg force-pushed the lint branch 2 times, most recently from 100022b to ee11ec8 Compare May 20, 2020 11:54

murali-reddy approved these changes May 28, 2020

View reviewed changes

aauren requested changes May 29, 2020

View reviewed changes

mrueg added 7 commits May 31, 2020 18:03

Makefile: Add lint using golangci-lint

0b267a2

build/travis-test.sh: Run lint step

8e934eb

metrics_controller: Lint

28927a3

pkg/metrics/metrics_controller.go:150:2: `mu` is unused (structcheck) mu sync.Mutex ^ pkg/metrics/metrics_controller.go:151:2: `nodeIP` is unused (structcheck) nodeIP net.IP ^

ecmp_vip: Lint

156f2cd

pkg/controllers/routing/ecmp_vip.go:208:4: S1023: redundant `return` statement (gosimple) return ^

bgp_peers: Lint

aa6cea5

pkg/controllers/routing/bgp_peers.go:331:4: S1023: redundant `return` statement (gosimple) return ^

ipset: Lint

46b1967

pkg/utils/ipset.go:243:23: Error return value of `entry.Set.Parent.Save` is not checked (errcheck) entry.Set.Parent.Save() ^

mrueg and others added 17 commits May 31, 2020 18:03

kube-router_test: Lint

568e750

cmd/kube-router/kube-router_test.go:21:10: Error return value of `io.Copy` is not checked (errcheck) io.Copy(stderrBuf, stderrR) ^ cmd/kube-router/kube-router_test.go:40:17: Error return value of `docBuf.ReadFrom` is not checked (errcheck) docBuf.ReadFrom(docF) ^

aws: Lint

c88c8b2

pkg/controllers/routing/aws.go:31:8: SA4006: this value of `err` is never used (staticcheck) URL, err := url.Parse(providerID) ^

network_services_controller_test: Lint

b70bac5

pkg/controllers/proxy/network_services_controller_test.go:80:6: func `logf` is unused (unused)

.golangci.yml: Increase timeout

4689aaf

Default is 1m, increase to 5m otherwise travis might fail

kube-router_test.go: defer waitgroup

db5a8b6

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>

Makefile: Update golangci-lint to 1.27.0

ad685f5

network_routes_controller: Incorporate review

4c8b6a9

mrueg force-pushed the lint branch from 27518ad to 3c6f03c Compare May 31, 2020 16:03

bgp_policies: Incorporate review

59d842d

mrueg force-pushed the lint branch from 3c6f03c to 59d842d Compare June 1, 2020 11:05

mrueg added 2 commits June 1, 2020 13:20

network_routes_controller: Incorporate review

2ed64eb

bgp_policies: Log error instead

4f982f3

mrueg force-pushed the lint branch from 862a7f7 to 4f982f3 Compare June 1, 2020 11:23

network_services_controller: Incorporate review

6be07fa

aauren approved these changes Jun 1, 2020

View reviewed changes

mrueg merged commit 12674d5 into cloudnativelabs:master Jun 3, 2020

Add golangci-lint support #895

Add golangci-lint support #895

Uh oh!

Conversation

mrueg commented May 5, 2020

Uh oh!

mrueg commented May 5, 2020

Uh oh!

mrueg left a comment

Choose a reason for hiding this comment

Uh oh!

mrueg May 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

murali-reddy commented May 28, 2020

Uh oh!

aauren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aauren May 29, 2020

Choose a reason for hiding this comment

Uh oh!

mrueg May 31, 2020

Choose a reason for hiding this comment

Uh oh!

murali-reddy Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aauren May 29, 2020

Choose a reason for hiding this comment

Uh oh!

aauren May 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aauren commented Jun 1, 2020

Uh oh!

murali-reddy commented Jun 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

murali-reddy Jun 1, 2020 •

edited

Loading