Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0b267a2
Makefile: Add lint using golangci-lint
mrueg May 4, 2020
8e934eb
build/travis-test.sh: Run lint step
mrueg May 5, 2020
cf58c0d
network_service_graceful: Lint
mrueg May 10, 2020
28927a3
metrics_controller: Lint
mrueg May 10, 2020
156f2cd
ecmp_vip: Lint
mrueg May 10, 2020
aa6cea5
bgp_peers: Lint
mrueg May 10, 2020
46b1967
ipset: Lint
mrueg May 10, 2020
568e750
kube-router_test: Lint
mrueg May 10, 2020
067ec12
cmd/kube-router/kube-router: Lint
mrueg May 10, 2020
c515adc
node: Lint
mrueg May 10, 2020
585ac0f
bgp_policies: Lint
mrueg May 10, 2020
41f62a0
network_policy_controller: Lint
mrueg May 10, 2020
c88c8b2
aws: Lint
mrueg May 10, 2020
b70bac5
network_services_controller_test: Lint
mrueg May 10, 2020
373008b
network_services_controller: Lint
mrueg May 10, 2020
9447fc2
service_endpoints_sync: Lint
mrueg May 10, 2020
210ed96
pkg/cmd/kube-router: Lint
mrueg May 10, 2020
9fe9335
network_routes_controller_test: Lint
mrueg May 10, 2020
6791567
network_routes_controller: Lint
mrueg May 10, 2020
127d551
health_controller: Lint
mrueg May 10, 2020
4689aaf
.golangci.yml: Increase timeout
mrueg May 10, 2020
db5a8b6
kube-router_test.go: defer waitgroup
mrueg May 31, 2020
ad685f5
Makefile: Update golangci-lint to 1.27.0
mrueg May 31, 2020
4c8b6a9
network_routes_controller: Incorporate review
mrueg May 31, 2020
59d842d
bgp_policies: Incorporate review
mrueg May 31, 2020
2ed64eb
network_routes_controller: Incorporate review
mrueg Jun 1, 2020
4f982f3
bgp_policies: Log error instead
mrueg Jun 1, 2020
6be07fa
network_services_controller: Incorporate review
mrueg Jun 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
run:
timeout: 5m
13 changes: 11 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ MAKEFILE_DIR=$(dir $(realpath $(firstword $(MAKEFILE_LIST))))
UPSTREAM_IMPORT_PATH=$(GOPATH)/src/github.com/cloudnativelabs/kube-router/
BUILD_IN_DOCKER?=true
DOCKER_BUILD_IMAGE?=golang:1.10.8-alpine3.9
DOCKER_LINT_IMAGE?=golangci/golangci-lint:v1.27.0
QEMU_IMAGE?=multiarch/qemu-user-static
ifeq ($(GOARCH), arm)
ARCH_TAG_PREFIX=$(GOARCH)
Expand Down Expand Up @@ -57,14 +58,22 @@ else
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
endif

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

lint: gofmt
ifeq "$(BUILD_IN_DOCKER)" "true"
$(DOCKER) run -v $(PWD):/go/src/github.com/cloudnativelabs/kube-router -w /go/src/github.com/cloudnativelabs/kube-router $(DOCKER_LINT_IMAGE) \
sh -c 'golangci-lint run ./...'
else
golangci-lint run ./...
endif

vagrant-up: export docker=$(DOCKER)
vagrant-up: export DEV_IMG=$(REGISTRY_DEV):$(IMG_TAG)
vagrant-up: all vagrant-destroy
Expand Down Expand Up @@ -264,7 +273,7 @@ ifeq (vagrant,$(firstword $(MAKECMDGOALS)))
endif

.PHONY: build clean container run release goreleaser push gofmt gofmt-fix gomoqs
.PHONY: update-glide test docker-login push-manifest push-manifest-release
.PHONY: update-glide test lint docker-login push-manifest push-manifest-release
.PHONY: push-release github-release help gopath gopath-fix vagrant-up-single-node
.PHONY: vagrant-up-multi-node vagrant-destroy vagrant-clean vagrant
.PHONY: multiarch-binverify
Expand Down
3 changes: 3 additions & 0 deletions build/travis-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@ set -o pipefail
echo "install moq"
go get github.com/matryer/moq

echo "Running lint on Travis"
make lint

echo "Running tests on Travis"
make test
18 changes: 13 additions & 5 deletions cmd/kube-router/kube-router.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@ func Main() error {

// Workaround for this issue:
// https://github.com/kubernetes/kubernetes/issues/17162
flag.CommandLine.Parse([]string{})

flag.Set("logtostderr", "true")
flag.Set("v", config.VLevel)
err := flag.CommandLine.Parse([]string{})
if err != nil {
return fmt.Errorf("Failed to parse flags: %s", err)
}
err = flag.Set("logtostderr", "true")
if err != nil {
return fmt.Errorf("Failed to set flag: %s", err)
}
err = flag.Set("v", config.VLevel)
if err != nil {
return fmt.Errorf("Failed to set flag: %s", err)
}

if config.HelpRequested {
pflag.Usage()
Expand Down Expand Up @@ -59,7 +67,7 @@ func Main() error {

if config.EnablePprof {
go func() {
fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error())
fmt.Fprintf(os.Stdout, "%s\n", http.ListenAndServe("0.0.0.0:6060", nil).Error())
}()
}

Expand Down
12 changes: 9 additions & 3 deletions cmd/kube-router/kube-router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ func TestMainHelp(t *testing.T) {
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
io.Copy(stderrBuf, stderrR)
wg.Done()
defer wg.Done()
_, err := io.Copy(stderrBuf, stderrR)
if err != nil {
panic(err)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}()

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

exp := append([]byte("```\n"), stderrBuf.Bytes()...)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/kube-router.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (kr *KubeRouter) Run() error {
}

// Handle SIGINT and SIGTERM
ch := make(chan os.Signal)
ch := make(chan os.Signal, 1)
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
<-ch

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

if !logOutput {
fmt.Fprintf(os.Stderr, output)
fmt.Fprintf(os.Stderr, "%s", output)
} else {
glog.Info(output)
}
Expand Down
20 changes: 18 additions & 2 deletions pkg/controllers/netpol/network_policy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
)

const (
networkPolicyAnnotation = "net.beta.kubernetes.io/network-policy"
kubePodFirewallChainPrefix = "KUBE-POD-FW-"
kubeNetworkPolicyChainPrefix = "KUBE-NWPLCY-"
kubeSourceIpSetPrefix = "KUBE-SRC-"
Expand Down Expand Up @@ -968,6 +967,9 @@ func cleanupStaleRules(activePolicyChains, activePodFwChains, activePolicyIPSets

// 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@murali-reddy murali-reddy Jun 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
for _, chain := range chains {
if strings.HasPrefix(chain, kubeNetworkPolicyChainPrefix) {
if _, ok := activePolicyChains[chain]; !ok {
Expand Down Expand Up @@ -1035,7 +1037,7 @@ func cleanupStaleRules(activePolicyChains, activePodFwChains, activePolicyIPSets
for podFwChain := range activePodFwChains {
podFwChainRules, err := iptablesCmdHandler.List("filter", podFwChain)
if err != nil {

return fmt.Errorf("Unable to list rules from the chain %s: %s", podFwChain, err)
}
for i, rule := range podFwChainRules {
if strings.Contains(rule, policyChain) {
Expand Down Expand Up @@ -1557,6 +1559,9 @@ func (npc *NetworkPolicyController) Cleanup() {
for i, rule := range forwardChainRules {
if strings.Contains(rule, kubePodFirewallChainPrefix) {
err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo))
if err != nil {
glog.Errorf("Failed to delete iptables rule as part of cleanup: %s", err)
}
realRuleNo++
}
}
Expand All @@ -1573,12 +1578,19 @@ func (npc *NetworkPolicyController) Cleanup() {
for i, rule := range forwardChainRules {
if strings.Contains(rule, kubePodFirewallChainPrefix) {
err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo))
if err != nil {
glog.Errorf("Failed to delete iptables rule as part of cleanup: %s", err)
}
realRuleNo++
}
}

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

// flush and delete per network policy specific chain
chains, err = iptablesCmdHandler.ListChains("filter")
if err != nil {
glog.Errorf("Unable to list chains: %s", err)
return
}
for _, chain := range chains {
if strings.HasPrefix(chain, kubeNetworkPolicyChainPrefix) {
err = iptablesCmdHandler.ClearChain("filter", chain)
Expand Down
5 changes: 0 additions & 5 deletions pkg/controllers/proxy/network_service_graceful.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ type gracefulQueue struct {
queue []gracefulRequest
}

type gracefulQueueItem struct {
added time.Time
service *ipvs.Service
}

type gracefulRequest struct {
ipvsSvc *ipvs.Service
ipvsDst *ipvs.Destination
Expand Down
Loading