refactor!: rewrite and cleanup kubenurse server code (#29)

* refactor!: rewrite and cleanup kubenurse server code

By using a package and multiple separate files the code is easier to
understand and test. A new /ready handler was added so we can configure
a readiness probe to allow seamless updates of kubenurse.

* build: update golangci-lint version

* build: update golangci-lint timeout, default is too short

* build: extract lint step and use go version 1.17

* feat: configure new readinessprobe in kustomize and helm templates

* fix: linter errors

* chore: cleanup, remove not needed WaitGroup

* refactor!: move pkg/kubediscovery to internal/kubediscovery

* refactor!: move pkg/checker to internal/servicecheck

* refactor!: incorporate pkg/metrics in internal/servicecheck

* refactor!: more refactorings to allow easier unit testing

* feat: more unit tests and coverage calculation in workflows

* docs: include ci and coverage badges in readme

* docs: fix coverage status URL

Co-authored-by: Clément Nussbaumer <[email protected]>

* chore: embed servicecheck.Result in /alive output for simplicity

Co-authored-by: Clément Nussbaumer <[email protected]>

Author: ghouscht

.github/workflows/ci-helm-deploy.yml

@@ -1,5 +1,5 @@
 ---
-name: ci-helm-deploy
+name: deploy with helm
 on:
   push:
   pull_request:
@@ -11,10 +11,8 @@ jobs:
         uses: actions/checkout@v2
       - name: Setup Go
         uses: actions/setup-go@v2
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.32
+          go-version: 1.17
       - name: GoReleaser
         uses: goreleaser/goreleaser-action@v2
         with:

.github/workflows/ci-kustomize-deploy.yml

@@ -1,5 +1,5 @@
 ---
-name: ci-kustomize-deploy
+name: deploy with kustomize
 on:
   push:
   pull_request:
@@ -11,10 +11,8 @@ jobs:
         uses: actions/checkout@v2
       - name: Setup Go
         uses: actions/setup-go@v2
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.32
+          go-version: 1.17
       - name: GoReleaser
         uses: goreleaser/goreleaser-action@v2
         with:

.github/workflows/helm-lint.yml

@@ -0,0 +1,37 @@
+---
+name: lint and test
+on:
+  push:
+  pull_request:
+jobs:
+  lint-go:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: golangci/golangci-lint-action@v2
+      with:
+        version: v1.43
+        args: --timeout 5m
+  lint-helm:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Helm lint
+      shell: bash
+      run: |
+        helm lint ./helm/kubenurse/
+  test-go:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-go@v2
+      with:
+        go-version: 1.17
+    - name: Run unit tests
+      run: go test -race -covermode atomic -coverprofile=profile.cov ./...
+    - name: Send coverage report
+      env:
+        COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      run: |
+        go install github.com/mattn/[email protected]
+        goveralls -coverprofile=profile.cov -service=github

.github/workflows/lint.yml

@@ -12,10 +12,8 @@ jobs:
         uses: actions/checkout@v2
       - name: Setup Go
         uses: actions/setup-go@v2
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.32
+          go-version: 1.17
       - name: Login to DockerHub
         uses: docker/login-action@v1
         with:

.github/workflows/release.yml

@@ -1,19 +1,20 @@
-# options for analysis running
+---
 run:
-  tests: true
-  timeout: 3m
-
-
-# all available settings of specific linters
+  tests: false
+  skip-dirs:
+    - .github
+    - build
+    - web
+    - .go
 linters-settings:
   dupl:
-    threshold: 150
+    threshold: 100
   funlen:
     lines: 100
     statements: 50
   goconst:
     min-len: 2
-    min-occurrences: 3
+    min-occurrences: 2
   gocritic:
     enabled-tags:
       - diagnostic
@@ -23,21 +24,27 @@ linters-settings:
       - style
     disabled-checks:
       - whyNoLint
+      - hugeParam
   gocyclo:
     min-complexity: 15
-  golint:
+  revive:
     min-confidence: 0.8
   govet:
     check-shadowing: true
-  maligned:
-    suggest-new: true
+  lll:
+    line-length: 140
   misspell:
     locale: UK
+  nolintlint:
+    allow-leading-space: false
+    require-explanation: true
+    allow-no-explanation:
+      - gocognit
+      - funlen
+      - gocyclo
 
 linters:
-  disable:
-    - gomnd
-    - lll
+  disable-all: true
   enable:
     - bodyclose
     - deadcode
@@ -46,28 +53,26 @@ linters:
     - dupl
     - errcheck
     - funlen
+    - nolintlint
     - gochecknoglobals
     - gochecknoinits
     - gocognit
     - goconst
     - gocritic
     - gocyclo
-    - godox
     - gofmt
     - goimports
-    - golint
+    - revive
     - goprintffuncname
     - gosec
     - gosimple
     - govet
     - ineffassign
-    - interfacer
-    - maligned
     - misspell
     - nakedret
     - prealloc
     - rowserrcheck
-    - scopelint # todo
+    - exportloopref
     - staticcheck
     - structcheck
     - stylecheck
@@ -78,12 +83,11 @@ linters:
     - varcheck
     - whitespace
     - wsl
-
-
 issues:
   exclude:
     # Very commonly not checked.
-    - 'Error return value of .(l.Sync|.*Close|.*Flush|os\.Remove(All)?|os\.(Un)?Setenv). is not checked'
+    - 'Error return value of .(l.Sync|.*Close|.*.Write|.*Flush|os\.Remove(All)?|os\.(Un)?Setenv). is not checked'
+    - 'G104:.*'
     - 'exported method (.*\.MarshalJSON|.*\.UnmarshalJSON) should have comment or be unexported'
     - 'shadow: declaration of "err" shadows declaration.*'
   max-same-issues: 0

.golangci.yml

@@ -1,3 +1,6 @@
+[![CI](https://github.com/postfinance/kubenurse/actions/workflows/release.yml/badge.svg)](https://github.com/postfinance/kubenurse/actions/workflows/release.yml)
+[![Coverage Status](https://coveralls.io/repos/github/postfinance/kubenurse/badge.svg?branch=master)](https://coveralls.io/github/postfinance/kubenurse?branch=master)
+
 # Kubenurse
 kubenurse is a little service that monitors all network connections in a kubernetes
 cluster and exports the taken metrics as prometheus endpoint.

README.md

@@ -36,6 +36,15 @@ spec:
         ports:
         - containerPort: 8080
           protocol: TCP
+        readinessProbe:
+          failureThreshold: 1
+          httpGet:
+            path: /ready
+            port: 8080
+            scheme: HTTP
+          periodSeconds: 3
+          successThreshold: 1
+          timeoutSeconds: 1
       tolerations:
       - effect: NoSchedule
         key: node-role.kubernetes.io/master

examples/daemonset.yaml

@@ -1,6 +1,6 @@
 module github.com/postfinance/kubenurse
 
-go 1.15
+go 1.17
 
 require (
 	github.com/prometheus/client_golang v1.11.0
@@ -9,3 +9,41 @@ require (
 	k8s.io/apimachinery v0.22.2
 	k8s.io/client-go v0.22.2
 )
+
+require (
+	github.com/beorn7/perks v1.0.1 // indirect
+	github.com/cespare/xxhash/v2 v2.1.1 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/evanphx/json-patch v4.11.0+incompatible // indirect
+	github.com/go-logr/logr v0.4.0 // indirect
+	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/golang/protobuf v1.5.2 // indirect
+	github.com/google/go-cmp v0.5.5 // indirect
+	github.com/google/gofuzz v1.1.0 // indirect
+	github.com/googleapis/gnostic v0.5.5 // indirect
+	github.com/json-iterator/go v1.1.11 // indirect
+	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.1 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/prometheus/client_model v0.2.0 // indirect
+	github.com/prometheus/common v0.26.0 // indirect
+	github.com/prometheus/procfs v0.6.0 // indirect
+	golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 // indirect
+	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect
+	golang.org/x/sys v0.0.0-20210616094352-59db8d763f22 // indirect
+	golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect
+	golang.org/x/text v0.3.6 // indirect
+	golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
+	google.golang.org/appengine v1.6.5 // indirect
+	google.golang.org/protobuf v1.26.0 // indirect
+	gopkg.in/inf.v0 v0.9.1 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
+	k8s.io/klog/v2 v2.9.0 // indirect
+	k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e // indirect
+	k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a // indirect
+	sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
+	sigs.k8s.io/yaml v1.2.0 // indirect
+)

go.mod

@@ -45,6 +45,15 @@ spec:
         ports:
         - containerPort: 8080
           protocol: TCP
+        readinessProbe:
+          failureThreshold: 1
+          httpGet:
+            path: /ready
+            port: 8080
+            scheme: HTTP
+          periodSeconds: 3
+          successThreshold: 1
+          timeoutSeconds: 1
       tolerations:
       - effect: NoSchedule
         key: node-role.kubernetes.io/master

helm/kubenurse/templates/daemonset.yaml

@@ -8,7 +8,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/rest"
 )
 
 // Client provides the kubediscovery client methods.
@@ -42,19 +41,11 @@ type Neighbour struct {
 // New creates a new kubediscovery client. The context is used to stop the k8s watchers/informers.
 // When allowUnschedulable is true, no node watcher is created and kubenurses
 // on unschedulable nodes are considered as neighbours.
-func New(ctx context.Context, allowUnschedulable bool) (*Client, error) {
-	// create in-cluster config
-	config, err := rest.InClusterConfig()
-	if err != nil {
-		return nil, fmt.Errorf("creating in-cluster configuration: %w", err)
-	}
-
-	cliset, err := kubernetes.NewForConfig(config)
-	if err != nil {
-		return nil, fmt.Errorf("creating clientset: %w", err)
-	}
-
-	var nc *nodeCache
+func New(ctx context.Context, cliset kubernetes.Interface, allowUnschedulable bool) (*Client, error) {
+	var (
+		nc  *nodeCache
+		err error
+	)
 
 	// Watch nodes only if we do not consider kubenurses on unschedulable nodes
 	if !allowUnschedulable {

pkg/kubediscovery/kubediscovery.go renamed to internal/kubediscovery/kubediscovery.go

@@ -0,0 +1,55 @@
+package kubediscovery
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+var (
+	kubenursePod = v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "kubenurse-dummy",
+			Labels: map[string]string{
+				"app": "kubenurse",
+			},
+		},
+	}
+	differentPod = v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "different",
+			Labels: map[string]string{
+				"app": "different",
+			},
+		},
+	}
+)
+
+func TestGetNeighbours(t *testing.T) {
+	r := require.New(t)
+	fakeClient := fake.NewSimpleClientset()
+
+	createFakePods(fakeClient)
+
+	client, err := New(context.Background(), fakeClient, false)
+	r.NoError(err)
+
+	neighbours, err := client.GetNeighbours(context.Background(), "kube-system", "app=kubenurse")
+	r.NoError(err)
+	r.Len(neighbours, 1)
+	r.Equal(kubenursePod.ObjectMeta.Name, neighbours[0].PodName)
+}
+
+func createFakePods(k8s kubernetes.Interface) {
+	for _, pod := range []v1.Pod{kubenursePod, differentPod} {
+		_, err := k8s.CoreV1().Pods("kube-system").Create(context.Background(), &pod, metav1.CreateOptions{})
+		if err != nil {
+			panic(err)
+		}
+	}
+}