Skip to content

Commit

Permalink
switch to golangci-lint and fixes new lint issues.
Browse files Browse the repository at this point in the history
  • Loading branch information
Cyril TOVENA authored and markmandel committed Sep 8, 2018
1 parent 7fed6b4 commit 2d8c351
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 42 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
!.helmignore
!.gitattributes
!.dockerignore
!.golangci.yml
*.iml
bin
*.o
Expand Down
63 changes: 63 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion build/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions build/build-image/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 && \
Expand Down
18 changes: 0 additions & 18 deletions build/build-image/gen-lint-exclude.sh

This file was deleted.

2 changes: 1 addition & 1 deletion examples/simple-udp/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/fleets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
3 changes: 3 additions & 0 deletions pkg/fleets/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:goconst
package fleets

import (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/gameservers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
17 changes: 8 additions & 9 deletions pkg/gameservers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import (
"k8s.io/client-go/tools/cache"
)

const ipFixture = "12.12.12.12"

func TestControllerSyncGameServer(t *testing.T) {
t.Parallel()

Expand All @@ -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}}}}

Expand Down Expand Up @@ -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)
})
})
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/gameservers/localsdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -115,6 +116,7 @@ func TestLocalSDKServerSetLabel(t *testing.T) {
}
}

// nolint:dupl
func TestLocalSDKServerSetAnnotation(t *testing.T) {

fixtures := map[string]struct {
Expand Down
4 changes: 2 additions & 2 deletions pkg/gameservers/portallocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/gameservers/sdkserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}()
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion sdks/go/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sdks/go/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 2d8c351

Please sign in to comment.