Switch to golangci-lint

.gitignore

@@ -17,6 +17,7 @@
 !.helmignore
 !.gitattributes
 !.dockerignore
+!.golangci.yml
 *.iml
 bin
 *.o

.golangci.yml

@@ -0,0 +1,63 @@
+# This file contains all available configuration options
+# with their default values.
+
+# options for analysis running
+run:
+  # default concurrency is a available CPU number
+  concurrency: 4
+
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 5m
+
+  # exit code when at least one issue was found, default is 1
+  issues-exit-code: 1
+
+  # include test files or not, default is true
+  tests: true
+
+  # list of build tags, all linters use it. Default is empty list.
+  build-tags:
+
+  # which dirs to skip: they won't be analyzed;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but next dirs are always skipped independently
+  # from this option's value:
+  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs:
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  skip-files:
+
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
+  format: colored-line-number
+
+  # print lines of code with issue, default is true
+  print-issued-lines: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+
+linters:
+  enable:
+    - megacheck
+    - govet
+    - golint
+    - interfacer
+    - unconvert
+    - dupl
+    - goconst
+    - gocyclo
+    - goimports
+    - maligned
+    - megacheck
+    - misspell
+    - unparam
+    - nakedret
+

build/Makefile

@@ -155,7 +155,7 @@ build-controller-binary: $(ensure-build-image)
 lint: LINT_TIMEOUT ?= 15m
 lint: $(ensure-build-image)
 	docker run --rm $(common_mounts) -w $(mount_path) $(DOCKER_RUN_ARGS) $(build_tag) bash -c \
-		"/root/gen-lint-exclude.sh && gometalinter --config .exclude.gometalinter.json --deadline=$(LINT_TIMEOUT) -t --skip vendor ./..."
+		"golangci-lint run ./examples/... && golangci-lint run --deadline $(LINT_TIMEOUT) ./..."
 
 # Build the image for the gameserver controller
 build-controller-image: $(ensure-build-image) build-controller-binary

build/build-image/Dockerfile

@@ -65,9 +65,11 @@ RUN mkdir -p /go/src/github.com/golang && cd /go/src/github.com/golang && \
     go install github.com/golang/protobuf/protoc-gen-go
 
 # install go tooling for development, building and testing
-RUN go get -u github.com/golang/dep/cmd/dep && \
-    go get -u github.com/alecthomas/gometalinter && \
-    /go/bin/gometalinter --install
+RUN go get -u github.com/golang/dep/cmd/dep
+
+# install golang-ci linter
+RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | \
+    bash -s -- -b $GOPATH/bin v1.10.2
 
 # install the release branch of the code generator tools
 RUN mkdir -p /go/src && cd /go/src && mkdir -p k8s.io && cd k8s.io && \

examples/simple-udp/main.go

@@ -21,12 +21,12 @@ import (
 	"log"
 	"net"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
 	coresdk "agones.dev/agones/pkg/sdk"
 	"agones.dev/agones/sdks/go"
-	"strconv"
 )
 
 // main starts a UDP server that received 1024 byte sized packets at at time

pkg/fleets/controller.go

@@ -121,6 +121,7 @@ func NewController(
 // creationMutationHandler is the handler for the mutating webhook that sets the
 // the default values on the Fleet
 // Should only be called on fleet create operations.
+// nolint:dupl
 func (c *Controller) creationMutationHandler(review admv1beta1.AdmissionReview) (admv1beta1.AdmissionReview, error) {
 	c.logger.WithField("review", review).Info("creationMutationHandler")
 

pkg/fleets/controller_test.go

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// nolint:goconst
 package fleets
 
 import (
@@ -74,6 +75,7 @@ func TestControllerSyncFleet(t *testing.T) {
 		f.Spec.Strategy.Type = appsv1.RecreateDeploymentStrategyType
 		c, m := newFakeController()
 		gsSet := f.GameServerSet()
+
 		gsSet.ObjectMeta.Name = "gsSet1"
 		gsSet.ObjectMeta.UID = "4321"
 		gsSet.Spec.Replicas = f.Spec.Replicas
@@ -318,6 +320,7 @@ func TestControllerUpdateFleetStatus(t *testing.T) {
 	gsSet1.Status.AllocatedReplicas = 1
 
 	gsSet2 := fleet.GameServerSet()
+	// nolint:goconst
 	gsSet2.ObjectMeta.Name = "gsSet2"
 	gsSet2.Status.Replicas = 5
 	gsSet2.Status.ReadyReplicas = 5

pkg/gameservers/controller.go

@@ -149,6 +149,7 @@ func NewController(
 // creationMutationHandler is the handler for the mutating webhook that sets the
 // the default values on the GameServer
 // Should only be called on gameserver create operations.
+// nolint:dupl
 func (c *Controller) creationMutationHandler(review admv1beta1.AdmissionReview) (admv1beta1.AdmissionReview, error) {
 	c.logger.WithField("review", review).Info("creationMutationHandler")
 

pkg/gameservers/controller_test.go

@@ -41,6 +41,8 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
+const ipFixture = "12.12.12.12"
+
 func TestControllerSyncGameServer(t *testing.T) {
 	t.Parallel()
 
@@ -58,7 +60,6 @@ func TestControllerSyncGameServer(t *testing.T) {
 			},
 		}
 
-		ipFixture := "12.12.12.12"
 		node := corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"},
 			Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{Address: ipFixture, Type: corev1.NodeExternalIP}}}}
 
@@ -430,7 +431,7 @@ func TestControllerSyncGameServerPortAllocationState(t *testing.T) {
 	})
 
 	t.Run("GameServer with non zero deletion datetime", func(t *testing.T) {
-		testWithNonZeroDeletionTimestamp(t, v1alpha1.Shutdown, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
+		testWithNonZeroDeletionTimestamp(t, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
 			return c.syncGameServerPortAllocationState(fixture)
 		})
 	})
@@ -444,7 +445,6 @@ func TestControllerSyncGameServerCreatingState(t *testing.T) {
 		return fixture
 	}
 
-	ipFixture := "12.12.12.12"
 	node := corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{Address: ipFixture, Type: corev1.NodeExternalIP}}}}
 
 	t.Run("Syncing from Created State, with no issues", func(t *testing.T) {
@@ -563,7 +563,7 @@ func TestControllerSyncGameServerCreatingState(t *testing.T) {
 	})
 
 	t.Run("GameServer with non zero deletion datetime", func(t *testing.T) {
-		testWithNonZeroDeletionTimestamp(t, v1alpha1.Shutdown, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
+		testWithNonZeroDeletionTimestamp(t, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
 			return c.syncGameServerCreatingState(fixture)
 		})
 	})
@@ -654,7 +654,6 @@ func TestControllerApplyGameServerAddressAndPort(t *testing.T) {
 	t.Parallel()
 	c, m := newFakeController()
 
-	ipFixture := "12.12.12.12"
 	gsFixture := &v1alpha1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
 		Spec: newSingleContainerSpec(), Status: v1alpha1.GameServerStatus{State: v1alpha1.RequestReady}}
 	gsFixture.ApplyDefaults()
@@ -721,7 +720,7 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) {
 	}
 
 	t.Run("GameServer with non zero deletion datetime", func(t *testing.T) {
-		testWithNonZeroDeletionTimestamp(t, v1alpha1.Shutdown, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
+		testWithNonZeroDeletionTimestamp(t, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
 			return c.syncGameServerRequestReadyState(fixture)
 		})
 	})
@@ -762,7 +761,7 @@ func TestControllerSyncGameServerShutdownState(t *testing.T) {
 	})
 
 	t.Run("GameServer with non zero deletion datetime", func(t *testing.T) {
-		testWithNonZeroDeletionTimestamp(t, v1alpha1.Shutdown, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
+		testWithNonZeroDeletionTimestamp(t, func(c *Controller, fixture *v1alpha1.GameServer) (*v1alpha1.GameServer, error) {
 			return fixture, c.syncGameServerShutdownState(fixture)
 		})
 	})
@@ -911,11 +910,11 @@ func testNoChange(t *testing.T, state v1alpha1.State, f func(*Controller, *v1alp
 
 // testWithNonZeroDeletionTimestamp runs a test with a given state, but
 // the DeletionTimestamp set to Now()
-func testWithNonZeroDeletionTimestamp(t *testing.T, state v1alpha1.State, f func(*Controller, *v1alpha1.GameServer) (*v1alpha1.GameServer, error)) {
+func testWithNonZeroDeletionTimestamp(t *testing.T, f func(*Controller, *v1alpha1.GameServer) (*v1alpha1.GameServer, error)) {
 	c, mocks := newFakeController()
 	now := metav1.Now()
 	fixture := &v1alpha1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", DeletionTimestamp: &now},
-		Spec: newSingleContainerSpec(), Status: v1alpha1.GameServerStatus{State: state}}
+		Spec: newSingleContainerSpec(), Status: v1alpha1.GameServerStatus{State: v1alpha1.Shutdown}}
 	fixture.ApplyDefaults()
 	updated := false
 	mocks.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {

pkg/gameservers/localsdk_test.go

@@ -70,14 +70,15 @@ func TestLocalSDKWithGameServer(t *testing.T) {
 	assert.Equal(t, fixture.ObjectMeta.Name, gs.ObjectMeta.Name)
 }
 
+// nolint:dupl
 func TestLocalSDKServerSetLabel(t *testing.T) {
 	fixtures := map[string]struct {
 		gs *v1alpha1.GameServer
 	}{
 		"default": {
 			gs: nil,
 		},
-		"no annotation": {
+		"no labels": {
 			gs: &v1alpha1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "empty"}},
 		},
 		"empty": {
@@ -115,6 +116,7 @@ func TestLocalSDKServerSetLabel(t *testing.T) {
 	}
 }
 
+// nolint:dupl
 func TestLocalSDKServerSetAnnotation(t *testing.T) {
 
 	fixtures := map[string]struct {

pkg/gameservers/portallocator_test.go

@@ -120,7 +120,7 @@ func TestPortAllocatorAllocate(t *testing.T) {
 			// ports between 10 and 20
 			for i := 10; i <= 20; i++ {
 				var p int32
-				gs, err := pa.Allocate(fixture.DeepCopy()) // nolint: vetshadow
+				gs, err := pa.Allocate(fixture.DeepCopy())
 				assert.True(t, 10 <= gs.Spec.Ports[0].HostPort && gs.Spec.Ports[0].HostPort <= 20, "%v is not between 10 and 20", p)
 				assert.Nil(t, err)
 			}
@@ -159,7 +159,7 @@ func TestPortAllocatorAllocate(t *testing.T) {
 			for i := 10; i <= 14; i++ {
 				copy := morePortFixture.DeepCopy()
 				copy.ObjectMeta.UID = types.UID(strconv.Itoa(x) + ":" + strconv.Itoa(i))
-				gs, err := pa.Allocate(copy) // nolint: vetshadow
+				gs, err := pa.Allocate(copy)
 				logrus.WithField("uid", copy.ObjectMeta.UID).WithField("ports", gs.Spec.Ports).WithError(err).Info("Allocated Port")
 				assert.Nil(t, err)
 				for _, p := range gs.Spec.Ports {

pkg/gameservers/sdkserver_test.go

@@ -297,7 +297,7 @@ func TestSidecarHealthLastUpdated(t *testing.T) {
 	wg := sync.WaitGroup{}
 	wg.Add(1)
 	go func() {
-		err := sc.Health(stream) // nolint: vetshadow
+		err := sc.Health(stream)
 		assert.Nil(t, err)
 		wg.Done()
 	}()
@@ -350,14 +350,14 @@ func TestSidecarHealthy(t *testing.T) {
 	wg := sync.WaitGroup{}
 	wg.Add(1)
 	go func() {
-		err := sc.Health(stream) // nolint: vetshadow
+		err := sc.Health(stream)
 		assert.Nil(t, err)
 		wg.Done()
 	}()
 
 	fixtures := map[string]struct {
-		disabled        bool
 		timeAdd         time.Duration
+		disabled        bool
 		expectedHealthy bool
 	}{
 		"disabled, under timeout": {disabled: true, timeAdd: time.Second, expectedHealthy: true},
@@ -620,7 +620,7 @@ func waitConnectedStreamCount(sc *SDKServer, count int) error {
 	})
 }
 
-func asyncWatchGameServer(t *testing.T, sc *SDKServer, stream *gameServerMockStream) {
+func asyncWatchGameServer(t *testing.T, sc *SDKServer, stream sdk.SDK_WatchGameServerServer) {
 	go func() {
 		err := sc.WatchGameServer(&sdk.Empty{}, stream)
 		assert.Nil(t, err)

sdks/go/sdk.go

@@ -17,13 +17,13 @@ package sdk
 
 import (
 	"fmt"
+	"io"
 	"time"
 
 	"agones.dev/agones/pkg/sdk"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
-	"io"
 )
 
 const port = 59357

sdks/go/sdk_test.go

@@ -16,13 +16,13 @@ package sdk
 
 import (
 	"testing"
+	"time"
 
 	"agones.dev/agones/pkg/sdk"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
-	"time"
 )
 
 func TestSDK(t *testing.T) {

test/e2e/framework/framework.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package framework is a package helping setting up end-to-end testing accross a
+// Package framework is a package helping setting up end-to-end testing across a
 // Kubernetes cluster.
 package framework
 
@@ -28,6 +28,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
+
 	// required to use gcloud login see: https://github.com/kubernetes/client-go/issues/242
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"k8s.io/client-go/tools/clientcmd"

test/e2e/gameserver_test.go

@@ -51,6 +51,7 @@ func TestCreateConnect(t *testing.T) {
 	assert.Equal(t, reply, "ACK: Hello World !\n")
 }
 
+// nolint:dupl
 func TestSDKSetLabel(t *testing.T) {
 	t.Parallel()
 	gs := defaultGameServer()
@@ -82,6 +83,7 @@ func TestSDKSetLabel(t *testing.T) {
 	assert.NotEmpty(t, gs.ObjectMeta.Labels["stable.agones.dev/sdk-timestamp"])
 }
 
+// nolint:dupl
 func TestSDKSetAnnotation(t *testing.T) {
 	t.Parallel()
 	gs := defaultGameServer()