Added linting step in makefile using gometalinter and fixed linting errors.

EricFortin · EricFortin · commit a61fdc55532d · 2018-04-14T13:42:42.000-04:00
diff --git a/build/Makefile b/build/Makefile
@@ -98,7 +98,7 @@ build-images: build-controller-image build-agones-sdk-image
 build-sdks: build-sdk-cpp
 
 # Run all tests
-test: ensure-build-image test-go test-install-yaml
+test: ensure-build-image lint test-go test-install-yaml
 
 # Run go tests
 test-go:
@@ -131,6 +131,11 @@ build-controller-binary: ensure-build-image
 	docker run --rm -e "CGO_ENABLED=0" $(common_mounts) $(build_tag) go build \
 		-o $(mount_path)/cmd/controller/bin/controller -a $(go_version_flags) -installsuffix cgo $(agones_package)/cmd/controller
 
+# Lint the go source code.
+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 100s -t --skip vendor ./..."
+
 # Build the image for the gameserver controller
 build-controller-image: ensure-build-image build-controller-binary
 	docker build $(agones_path)/cmd/controller/ --tag=$(controller_tag) $(DOCKER_BUILD_ARGS)
diff --git a/build/build-image/Dockerfile b/build/build-image/Dockerfile
@@ -12,14 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# ForceUpdate 2 -- change here if you need to force a rebuild
+# ForceUpdate 3 -- change here if you need to force a rebuild
 
 # compiling proto + grpc takes an exceptionally long time
 # so we'll use that as the base.
 FROM grpc/cxx:1.8
 
 RUN apt-get update && \
-    apt-get install -y wget rsync make python bash-completion zip && \
+    apt-get install -y wget rsync make python bash-completion zip jq && \
     apt-get clean
 
 # install go
diff --git a/build/build-image/gen-lint-exclude.sh b/build/build-image/gen-lint-exclude.sh
@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+# Copyright 2018 Google Inc. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+excluded=$(find . -name "*.go" | grep -v vendor | xargs grep autogenerated | cut -d ':' -f 1 | uniq | sed 's#\./##g')
+jq -n --arg inarjq -n --arg inarr "${excluded}" '{ Exclude: ($inarr | split("\n")) }' > .exclude.gometalinter.json
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -56,7 +56,7 @@ var (
 )
 
 // main starts the operator for the gameserver CRD
-func main() {
+func main() { // nolint: gocyclo
 	exec, err := os.Executable()
 	if err != nil {
 		logger.WithError(err).Fatal("Could not get executable path")
diff --git a/examples/simple-udp/server/main.go b/examples/simple-udp/server/main.go
@@ -29,7 +29,7 @@ import (
 
 // main starts a UDP server that received 1024 byte sized packets at at time
 // converts the bytes to a string, and logs the output
-func main() {
+func main() { // nolint: gocyclo
 	port := flag.String("port", "7654", "The port to listen to udp traffic on")
 	flag.Parse()
 	if ep := os.Getenv("PORT"); ep != "" {
@@ -41,7 +41,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("Could not start udp server: %v", err)
 	}
-	defer conn.Close()
+	defer conn.Close() // nolint: errcheck
 
 	log.Print("Creating SDK instance")
 	s, err := sdk.NewSDK()
@@ -79,12 +79,10 @@ func main() {
 				log.Printf("Could not shutdown")
 			}
 			os.Exit(0)
-			break
 
 		// turns off the health pings
 		case "UNHEALTHY":
 			close(stop)
-			break
 		}
 
 		// echo it back
diff --git a/pkg/apis/stable/v1alpha1/fleet.go b/pkg/apis/stable/v1alpha1/fleet.go
@@ -20,6 +20,8 @@ import (
 )
 
 const (
+	// FleetGameServerSetLabel is the label that the name of the Fleet
+	// is set to on the GameServerSet the Fleet controls
 	FleetGameServerSetLabel = stable.GroupName + "/fleet"
 )
 
@@ -62,7 +64,7 @@ type FleetStatus struct {
 	ReadyReplicas int32 `json:"readyReplicas"`
 }
 
-// Fleet returns a single GameServerSet for this Fleet definition
+// GameServerSet returns a single GameServerSet for this Fleet definition
 func (f *Fleet) GameServerSet() *GameServerSet {
 	gsSet := &GameServerSet{
 		ObjectMeta: *f.Spec.Template.ObjectMeta.DeepCopy(),
diff --git a/pkg/gameservers/helper_test.go b/pkg/gameservers/helper_test.go
@@ -100,7 +100,7 @@ func testHTTPHealth(t *testing.T, url string, expectedResponse string, expectedS
 
 		assert.NotNil(t, resp)
 		if resp != nil {
-			defer resp.Body.Close()
+			defer resp.Body.Close() // nolint: errcheck
 			body, err := ioutil.ReadAll(resp.Body)
 			assert.Nil(t, err, "(%s) read response error should be nil: %v", url, err)
 			assert.Equal(t, expectedStatus, resp.StatusCode, "url: %s", url)
diff --git a/pkg/gameservers/sdkserver_test.go b/pkg/gameservers/sdkserver_test.go
@@ -41,13 +41,13 @@ func TestSidecarRun(t *testing.T) {
 		"ready": {
 			state: v1alpha1.RequestReady,
 			f: func(sc *SDKServer, ctx context.Context) {
-				sc.Ready(ctx, &sdk.Empty{})
+				sc.Ready(ctx, &sdk.Empty{}) // nolint: errcheck
 			},
 		},
 		"shutdown": {
 			state: v1alpha1.Shutdown,
 			f: func(sc *SDKServer, ctx context.Context) {
-				sc.Shutdown(ctx, &sdk.Empty{})
+				sc.Shutdown(ctx, &sdk.Empty{}) // nolint: errcheck
 			},
 		},
 		"unhealthy": {
diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go
@@ -159,7 +159,7 @@ func TestSyncGameServerSet(t *testing.T) {
 		_, cancel := agtesting.StartInformers(m, c.gameServerSetSynced, c.gameServerSynced)
 		defer cancel()
 
-		c.syncGameServerSet(gsSet.ObjectMeta.Namespace + "/" + gsSet.ObjectMeta.Name)
+		c.syncGameServerSet(gsSet.ObjectMeta.Namespace + "/" + gsSet.ObjectMeta.Name) // nolint: errcheck
 
 		assert.Equal(t, 5, count)
 		assert.True(t, deleted, "A game servers should have been deleted")
@@ -185,7 +185,7 @@ func TestSyncGameServerSet(t *testing.T) {
 		_, cancel := agtesting.StartInformers(m, c.gameServerSetSynced, c.gameServerSynced)
 		defer cancel()
 
-		c.syncGameServerSet(gsSet.ObjectMeta.Namespace + "/" + gsSet.ObjectMeta.Name)
+		c.syncGameServerSet(gsSet.ObjectMeta.Namespace + "/" + gsSet.ObjectMeta.Name) // nolint: errcheck
 
 		assert.Equal(t, 5, count)
 	})
diff --git a/pkg/gameserversets/helper_test.go b/pkg/gameserversets/helper_test.go
@@ -28,15 +28,15 @@ import (
 )
 
 // holder for all my fakes and mocks
-type mocks struct {
+type mocks struct { // nolint: megacheck
 	kubeClient            *kubefake.Clientset
 	extClient             *extfake.Clientset
 	agonesClient          *agonesfake.Clientset
 	agonesInformerFactory externalversions.SharedInformerFactory
 	fakeRecorder          *record.FakeRecorder
 }
 
-func newMocks() mocks {
+func newMocks() mocks { // nolint: megacheck
 	kubeClient := &kubefake.Clientset{}
 	extClient := &extfake.Clientset{}
 	agonesClient := &agonesfake.Clientset{}
@@ -51,7 +51,7 @@ func newMocks() mocks {
 	return m
 }
 
-func startInformers(mocks mocks, sync ...cache.InformerSynced) (<-chan struct{}, context.CancelFunc) {
+func startInformers(mocks mocks, sync ...cache.InformerSynced) (<-chan struct{}, context.CancelFunc) { // nolint: megacheck
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	stop := ctx.Done()
 
diff --git a/pkg/testing/controller.go b/pkg/testing/controller.go
@@ -31,7 +31,7 @@ import (
 
 // Handy tools for testing controllers
 
-// holder for all my fakes and Mocks
+// Mocks is a holder for all my fakes and Mocks
 type Mocks struct {
 	KubeClient             *kubefake.Clientset
 	KubeInformationFactory informers.SharedInformerFactory
@@ -41,6 +41,7 @@ type Mocks struct {
 	FakeRecorder           *record.FakeRecorder
 }
 
+// NewMocks creates a new set of fakes and mocks.
 func NewMocks() Mocks {
 	kubeClient := &kubefake.Clientset{}
 	agonesClient := &agonesfake.Clientset{}
@@ -56,6 +57,7 @@ func NewMocks() Mocks {
 	return m
 }
 
+// StartInformers starts new fake informers
 func StartInformers(mocks Mocks, sync ...cache.InformerSynced) (<-chan struct{}, context.CancelFunc) {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	stop := ctx.Done()
@@ -71,6 +73,7 @@ func StartInformers(mocks Mocks, sync ...cache.InformerSynced) (<-chan struct{},
 	return stop, cancel
 }
 
+// NewEstablishedCRD fakes CRD installation success.
 func NewEstablishedCRD() *v1beta1.CustomResourceDefinition {
 	return &v1beta1.CustomResourceDefinition{
 		Status: v1beta1.CustomResourceDefinitionStatus{
diff --git a/pkg/util/webhooks/webhooks.go b/pkg/util/webhooks/webhooks.go
@@ -108,7 +108,7 @@ func (wh *WebHook) AddHandler(path string, gk schema.GroupKind, op v1beta1.Opera
 }
 
 // handle Handles http requests for webhooks
-func (wh *WebHook) handle(path string, w http.ResponseWriter, r *http.Request) error {
+func (wh *WebHook) handle(path string, w http.ResponseWriter, r *http.Request) error { // nolint: interfacer
 	wh.logger.WithField("path", path).Info("running webhook")
 
 	var review v1beta1.AdmissionReview
diff --git a/pkg/util/workerqueue/workerqueue_test.go b/pkg/util/workerqueue/workerqueue_test.go
@@ -113,7 +113,7 @@ func TestWorkQueueHealthCheck(t *testing.T) {
 	f := func(t *testing.T, url string, status int) {
 		resp, err := http.Get(url)
 		assert.Nil(t, err)
-		defer resp.Body.Close()
+		defer resp.Body.Close() // nolint: errcheck
 
 		body, err := ioutil.ReadAll(resp.Body)
 		assert.Nil(t, err)
diff --git a/sdks/go/sdk_test.go b/sdks/go/sdk_test.go
@@ -39,14 +39,17 @@ func TestSDK(t *testing.T) {
 	assert.False(t, sm.shutdown)
 	assert.False(t, sm.hm.healthy)
 
-	s.Ready()
+	err := s.Ready()
+	assert.Nil(t, err)
 	assert.True(t, sm.ready)
 	assert.False(t, sm.shutdown)
 
-	s.Health()
+	err = s.Health()
+	assert.Nil(t, err)
 	assert.True(t, sm.hm.healthy)
 
-	s.Shutdown()
+	err = s.Shutdown()
+	assert.Nil(t, err)
 	assert.True(t, sm.ready)
 	assert.True(t, sm.shutdown)
 }

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ var (`
`56`	`56`	`)`
`57`	`57`
`58`	`58`	`// main starts the operator for the gameserver CRD`
`59`		`-func main() {`
	`59`	`+func main() { // nolint: gocyclo`
`60`	`60`	`exec, err := os.Executable()`
`61`	`61`	`if err != nil {`
`62`	`62`	`logger.WithError(err).Fatal("Could not get executable path")`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,8 @@ import (`
`20`	`20`	`)`
`21`	`21`
`22`	`22`	`const (`
	`23`	`+ // FleetGameServerSetLabel is the label that the name of the Fleet`
	`24`	`+ // is set to on the GameServerSet the Fleet controls`
`23`	`25`	`FleetGameServerSetLabel = stable.GroupName + "/fleet"`
`24`	`26`	`)`
`25`	`27`
`@@ -62,7 +64,7 @@ type FleetStatus struct {`
`62`	`64`	ReadyReplicas int32 `json:"readyReplicas"`
`63`	`65`	`}`
`64`	`66`
`65`		`-// Fleet returns a single GameServerSet for this Fleet definition`
	`67`	`+// GameServerSet returns a single GameServerSet for this Fleet definition`
`66`	`68`	`func (f Fleet) GameServerSet() GameServerSet {`
`67`	`69`	`gsSet := &GameServerSet{`
`68`	`70`	`ObjectMeta: *f.Spec.Template.ObjectMeta.DeepCopy(),`