Skip to content

Commit 12674d5

Browse files
mruegaauren
andauthored
Add golangci-lint support (#895)
* 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 <[email protected]> * 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 <[email protected]>
1 parent 4a08e11 commit 12674d5

21 files changed

+316
-120
lines changed

.golangci.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
run:
2+
timeout: 5m

Makefile

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ MAKEFILE_DIR=$(dir $(realpath $(firstword $(MAKEFILE_LIST))))
1919
UPSTREAM_IMPORT_PATH=$(GOPATH)/src/github.com/cloudnativelabs/kube-router/
2020
BUILD_IN_DOCKER?=true
2121
DOCKER_BUILD_IMAGE?=golang:1.10.8-alpine3.9
22+
DOCKER_LINT_IMAGE?=golangci/golangci-lint:v1.27.0
2223
QEMU_IMAGE?=multiarch/qemu-user-static
2324
ifeq ($(GOARCH), arm)
2425
ARCH_TAG_PREFIX=$(GOARCH)
@@ -57,14 +58,22 @@ else
5758
GOARCH=$(GOARCH) CGO_ENABLED=0 go build -ldflags '-X github.com/cloudnativelabs/kube-router/pkg/cmd.version=$(GIT_COMMIT) -X github.com/cloudnativelabs/kube-router/pkg/cmd.buildDate=$(BUILD_DATE)' -o kube-router cmd/kube-router/kube-router.go
5859
endif
5960

60-
test: gofmt ## Runs code quality pipelines (gofmt, tests, coverage, lint, etc)
61+
test: gofmt ## Runs code quality pipelines (gofmt, tests, coverage, etc)
6162
ifeq "$(BUILD_IN_DOCKER)" "true"
6263
$(DOCKER) run -v $(PWD):/go/src/github.com/cloudnativelabs/kube-router -w /go/src/github.com/cloudnativelabs/kube-router $(DOCKER_BUILD_IMAGE) \
6364
sh -c 'go test -v -timeout 30s github.com/cloudnativelabs/kube-router/cmd/kube-router/ github.com/cloudnativelabs/kube-router/pkg/...'
6465
else
6566
go test -v -timeout 30s github.com/cloudnativelabs/kube-router/cmd/kube-router/ github.com/cloudnativelabs/kube-router/pkg/...
6667
endif
6768

69+
lint: gofmt
70+
ifeq "$(BUILD_IN_DOCKER)" "true"
71+
$(DOCKER) run -v $(PWD):/go/src/github.com/cloudnativelabs/kube-router -w /go/src/github.com/cloudnativelabs/kube-router $(DOCKER_LINT_IMAGE) \
72+
sh -c 'golangci-lint run ./...'
73+
else
74+
golangci-lint run ./...
75+
endif
76+
6877
vagrant-up: export docker=$(DOCKER)
6978
vagrant-up: export DEV_IMG=$(REGISTRY_DEV):$(IMG_TAG)
7079
vagrant-up: all vagrant-destroy
@@ -264,7 +273,7 @@ ifeq (vagrant,$(firstword $(MAKECMDGOALS)))
264273
endif
265274

266275
.PHONY: build clean container run release goreleaser push gofmt gofmt-fix gomoqs
267-
.PHONY: update-glide test docker-login push-manifest push-manifest-release
276+
.PHONY: update-glide test lint docker-login push-manifest push-manifest-release
268277
.PHONY: push-release github-release help gopath gopath-fix vagrant-up-single-node
269278
.PHONY: vagrant-up-multi-node vagrant-destroy vagrant-clean vagrant
270279
.PHONY: multiarch-binverify

build/travis-test.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,8 @@ set -o pipefail
55
echo "install moq"
66
go get github.com/matryer/moq
77

8+
echo "Running lint on Travis"
9+
make lint
10+
811
echo "Running tests on Travis"
912
make test

cmd/kube-router/kube-router.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,18 @@ func Main() error {
2828

2929
// Workaround for this issue:
3030
// https://github.com/kubernetes/kubernetes/issues/17162
31-
flag.CommandLine.Parse([]string{})
32-
33-
flag.Set("logtostderr", "true")
34-
flag.Set("v", config.VLevel)
31+
err := flag.CommandLine.Parse([]string{})
32+
if err != nil {
33+
return fmt.Errorf("Failed to parse flags: %s", err)
34+
}
35+
err = flag.Set("logtostderr", "true")
36+
if err != nil {
37+
return fmt.Errorf("Failed to set flag: %s", err)
38+
}
39+
err = flag.Set("v", config.VLevel)
40+
if err != nil {
41+
return fmt.Errorf("Failed to set flag: %s", err)
42+
}
3543

3644
if config.HelpRequested {
3745
pflag.Usage()
@@ -59,7 +67,7 @@ func Main() error {
5967

6068
if config.EnablePprof {
6169
go func() {
62-
fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error())
70+
fmt.Fprintf(os.Stdout, "%s\n", http.ListenAndServe("0.0.0.0:6060", nil).Error())
6371
}()
6472
}
6573

cmd/kube-router/kube-router_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ func TestMainHelp(t *testing.T) {
1818
wg := &sync.WaitGroup{}
1919
wg.Add(1)
2020
go func() {
21-
io.Copy(stderrBuf, stderrR)
22-
wg.Done()
21+
defer wg.Done()
22+
_, err := io.Copy(stderrBuf, stderrR)
23+
if err != nil {
24+
panic(err)
25+
}
2326
}()
2427

2528
origArgs := os.Args
@@ -37,7 +40,10 @@ func TestMainHelp(t *testing.T) {
3740
t.Fatalf("could not open docs/user-guide.md: %s\n", err)
3841
}
3942
docBuf := bytes.NewBuffer(nil)
40-
docBuf.ReadFrom(docF)
43+
_, err = docBuf.ReadFrom(docF)
44+
if err != nil {
45+
t.Fatalf("could not read from buffer: %s\n", err)
46+
}
4147
docF.Close()
4248

4349
exp := append([]byte("```\n"), stderrBuf.Bytes()...)

pkg/cmd/kube-router.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func (kr *KubeRouter) Run() error {
180180
}
181181

182182
// Handle SIGINT and SIGTERM
183-
ch := make(chan os.Signal)
183+
ch := make(chan os.Signal, 1)
184184
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
185185
<-ch
186186

@@ -211,7 +211,7 @@ func PrintVersion(logOutput bool) {
211211
output := fmt.Sprintf("Running %v version %s, built on %s, %s\n", os.Args[0], version, buildDate, runtime.Version())
212212

213213
if !logOutput {
214-
fmt.Fprintf(os.Stderr, output)
214+
fmt.Fprintf(os.Stderr, "%s", output)
215215
} else {
216216
glog.Info(output)
217217
}

pkg/controllers/netpol/network_policy_controller.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
)
3333

3434
const (
35-
networkPolicyAnnotation = "net.beta.kubernetes.io/network-policy"
3635
kubePodFirewallChainPrefix = "KUBE-POD-FW-"
3736
kubeNetworkPolicyChainPrefix = "KUBE-NWPLCY-"
3837
kubeSourceIpSetPrefix = "KUBE-SRC-"
@@ -968,6 +967,9 @@ func cleanupStaleRules(activePolicyChains, activePodFwChains, activePolicyIPSets
968967

969968
// find iptables chains and ipsets that are no longer used by comparing current to the active maps we were passed
970969
chains, err := iptablesCmdHandler.ListChains("filter")
970+
if err != nil {
971+
return fmt.Errorf("Unable to list chains: %s", err)
972+
}
971973
for _, chain := range chains {
972974
if strings.HasPrefix(chain, kubeNetworkPolicyChainPrefix) {
973975
if _, ok := activePolicyChains[chain]; !ok {
@@ -1035,7 +1037,7 @@ func cleanupStaleRules(activePolicyChains, activePodFwChains, activePolicyIPSets
10351037
for podFwChain := range activePodFwChains {
10361038
podFwChainRules, err := iptablesCmdHandler.List("filter", podFwChain)
10371039
if err != nil {
1038-
1040+
return fmt.Errorf("Unable to list rules from the chain %s: %s", podFwChain, err)
10391041
}
10401042
for i, rule := range podFwChainRules {
10411043
if strings.Contains(rule, policyChain) {
@@ -1557,6 +1559,9 @@ func (npc *NetworkPolicyController) Cleanup() {
15571559
for i, rule := range forwardChainRules {
15581560
if strings.Contains(rule, kubePodFirewallChainPrefix) {
15591561
err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo))
1562+
if err != nil {
1563+
glog.Errorf("Failed to delete iptables rule as part of cleanup: %s", err)
1564+
}
15601565
realRuleNo++
15611566
}
15621567
}
@@ -1573,12 +1578,19 @@ func (npc *NetworkPolicyController) Cleanup() {
15731578
for i, rule := range forwardChainRules {
15741579
if strings.Contains(rule, kubePodFirewallChainPrefix) {
15751580
err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo))
1581+
if err != nil {
1582+
glog.Errorf("Failed to delete iptables rule as part of cleanup: %s", err)
1583+
}
15761584
realRuleNo++
15771585
}
15781586
}
15791587

15801588
// flush and delete pod specific firewall chain
15811589
chains, err := iptablesCmdHandler.ListChains("filter")
1590+
if err != nil {
1591+
glog.Errorf("Unable to list chains: %s", err)
1592+
return
1593+
}
15821594
for _, chain := range chains {
15831595
if strings.HasPrefix(chain, kubePodFirewallChainPrefix) {
15841596
err = iptablesCmdHandler.ClearChain("filter", chain)
@@ -1596,6 +1608,10 @@ func (npc *NetworkPolicyController) Cleanup() {
15961608

15971609
// flush and delete per network policy specific chain
15981610
chains, err = iptablesCmdHandler.ListChains("filter")
1611+
if err != nil {
1612+
glog.Errorf("Unable to list chains: %s", err)
1613+
return
1614+
}
15991615
for _, chain := range chains {
16001616
if strings.HasPrefix(chain, kubeNetworkPolicyChainPrefix) {
16011617
err = iptablesCmdHandler.ClearChain("filter", chain)

pkg/controllers/proxy/network_service_graceful.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@ type gracefulQueue struct {
1818
queue []gracefulRequest
1919
}
2020

21-
type gracefulQueueItem struct {
22-
added time.Time
23-
service *ipvs.Service
24-
}
25-
2621
type gracefulRequest struct {
2722
ipvsSvc *ipvs.Service
2823
ipvsDst *ipvs.Destination

0 commit comments

Comments
 (0)