diff --git a/.golangci.yml b/.golangci.yml index 67315c0b44..b8f7767a52 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -46,6 +46,7 @@ output: linters: enable: + - deadcode - megacheck - govet - golint @@ -53,16 +54,39 @@ linters: - unconvert - dupl - goconst + - gocritic - gocyclo - goimports + - interfacer - maligned - megacheck + - scopelint + - staticcheck + - structcheck - misspell - unparam - nakedret - bodyclose +linters-settings: + gocritic: + enabled-tags: + - performance + - opinionated + - diagnostic + disabled-checks: + - paramTypeCombine + - unnamedResult + settings: # settings passed to gocritic + hugeParam: + sizeThreshold: 512 + rangeValCopy: + sizeThreshold: 512 + skipTestFuncs: true + issues: # This turns off the default excludes - which was causing the linter # to miss things like erroneous comments - exclude-use-default: false \ No newline at end of file + exclude-use-default: false + exclude: + - Using the variable on range scope .* in function literal diff --git a/build/Makefile b/build/Makefile index 0bc59fa866..34ee0f7a97 100644 --- a/build/Makefile +++ b/build/Makefile @@ -304,7 +304,7 @@ build-controller-binary: $(ensure-build-image) lint: LINT_TIMEOUT ?= 15m lint: $(ensure-build-image) docker run -t -e "TERM=xterm-256color" -e "$(gomod_on)" --rm $(common_mounts) -w $(workdir_path) $(DOCKER_RUN_ARGS) $(build_tag) bash -c \ - "golangci-lint run ./examples/... && golangci-lint run --deadline $(LINT_TIMEOUT) ./..." + "golangci-lint run ./examples/... && golangci-lint run --timeout $(LINT_TIMEOUT) ./..." # Extract licenses from source tree build-licenses: diff --git a/build/build-image/Dockerfile b/build/build-image/Dockerfile index 3438b5514b..0c6d8fd1a2 100644 --- a/build/build-image/Dockerfile +++ b/build/build-image/Dockerfile @@ -68,7 +68,7 @@ RUN curl -L ${HELM_URL} > /tmp/helm.tar.gz \ RUN echo "source <(helm completion bash)" >> /root/.bashrc # install golang-ci linter -RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.18.0 +RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.22.2 # # \ \ / /__| |__ ___(_) |_ ___ diff --git a/cmd/allocator/main.go b/cmd/allocator/main.go index e262f42719..3fa65bdfa0 100644 --- a/cmd/allocator/main.go +++ b/cmd/allocator/main.go @@ -243,19 +243,20 @@ func getCACertPool(path string) (*x509.CertPool, error) { } for _, file := range filesInfo { - if strings.HasSuffix(file.Name(), ".crt") || strings.HasSuffix(file.Name(), ".pem") { - certFile := filepath.Join(path, file.Name()) - caCert, err := ioutil.ReadFile(certFile) - if err != nil { - logger.Errorf("ca cert is not readable or missing: %s", err.Error()) - continue - } - if !caCertPool.AppendCertsFromPEM(caCert) { - logger.Errorf("client cert %s cannot be installed", certFile) - continue - } - logger.Infof("client cert %s is installed", certFile) + if !strings.HasSuffix(file.Name(), ".crt") && !strings.HasSuffix(file.Name(), ".pem") { + continue + } + certFile := filepath.Join(path, file.Name()) + caCert, err := ioutil.ReadFile(certFile) + if err != nil { + logger.Errorf("ca cert is not readable or missing: %s", err.Error()) + continue + } + if !caCertPool.AppendCertsFromPEM(caCert) { + logger.Errorf("client cert %s cannot be installed", certFile) + continue } + logger.Infof("client cert %s is installed", certFile) } return caCertPool, nil @@ -323,8 +324,7 @@ func (h *httpHandler) allocateHandler(w http.ResponseWriter, r *http.Request) { func httpCode(err error) int { code := http.StatusInternalServerError - switch t := err.(type) { - case k8serror.APIStatus: + if t, ok := err.(k8serror.APIStatus); ok { code = int(t.Status().Code) } return code diff --git a/cmd/controller/main.go b/cmd/controller/main.go index d99d57b850..9d23e8b764 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -359,7 +359,7 @@ type config struct { } // validate ensures the ctlConfig data is valid. -func (c config) validate() error { +func (c *config) validate() error { if c.MinPort <= 0 || c.MaxPort <= 0 { return errors.New("min Port and Max Port values are required") } diff --git a/cmd/ping/udp_test.go b/cmd/ping/udp_test.go index 52b611874b..cf047ec20b 100644 --- a/cmd/ping/udp_test.go +++ b/cmd/ping/udp_test.go @@ -120,9 +120,9 @@ func TestUDPServerHealth(t *testing.T) { assert.Nil(t, err) } -func defaultFixture(clock clock.Clock) (*udpServer, error) { +func defaultFixture(cl clock.Clock) (*udpServer, error) { u := newUDPServer(5) - u.clock = clock + u.clock = cl var err error u.conn, err = net.ListenPacket("udp", ":0") return u, err diff --git a/cmd/sdk-server/main.go b/cmd/sdk-server/main.go index 85cc944e4b..e2ac0a963c 100644 --- a/cmd/sdk-server/main.go +++ b/cmd/sdk-server/main.go @@ -90,7 +90,8 @@ func main() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - if ctlConf.IsLocal { + switch { + case ctlConf.IsLocal: localSDK, err := registerLocal(grpcServer, ctlConf) if err != nil { logger.WithError(err).Fatal("Could not start local sdk server") @@ -103,7 +104,7 @@ func main() { close(timedStop) }() } - } else if ctlConf.Test != "" { + case ctlConf.Test != "": localSDK, err := registerTestSdkServer(grpcServer, ctlConf) if err != nil { logger.WithError(err).Fatal("Could not start test sdk server") @@ -116,7 +117,7 @@ func main() { close(timedStop) }() } - } else { + default: var config *rest.Config config, err := rest.InClusterConfig() if err != nil { diff --git a/pkg/allocation/converters/converter.go b/pkg/allocation/converters/converter.go index 96e401527c..d5bfada32c 100644 --- a/pkg/allocation/converters/converter.go +++ b/pkg/allocation/converters/converter.go @@ -127,6 +127,7 @@ func convertInternalLabelSelectorToLabelSelector(in *metav1.LabelSelector) *pb.L func convertInternalLabelSelectorsToLabelSelectors(in []metav1.LabelSelector) []*pb.LabelSelector { var result []*pb.LabelSelector for _, l := range in { + l := l c := convertInternalLabelSelectorToLabelSelector(&l) result = append(result, c) } diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index 02815cdd21..6a58cae4c3 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -74,7 +74,7 @@ func validateGSSpec(gs gsSpec) []metav1.StatusCause { // validateObjectMeta Check ObjectMeta specification // Used by Fleet, GameServerSet and GameServer -func validateObjectMeta(objMeta metav1.ObjectMeta) []metav1.StatusCause { +func validateObjectMeta(objMeta *metav1.ObjectMeta) []metav1.StatusCause { var causes []metav1.StatusCause errs := metav1validation.ValidateLabels(objMeta.Labels, field.NewPath("labels")) diff --git a/pkg/apis/agones/v1/fleet.go b/pkg/apis/agones/v1/fleet.go index 7e1b21a6a2..40b7669554 100644 --- a/pkg/apis/agones/v1/fleet.go +++ b/pkg/apis/agones/v1/fleet.go @@ -179,7 +179,7 @@ func (f *Fleet) Validate() ([]metav1.StatusCause, bool) { if len(gsCauses) > 0 { causes = append(causes, gsCauses...) } - objMetaCauses := validateObjectMeta(f.Spec.Template.ObjectMeta) + objMetaCauses := validateObjectMeta(&f.Spec.Template.ObjectMeta) if len(objMetaCauses) > 0 { causes = append(causes, objMetaCauses...) } diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index e3c5f7b210..bdc38d9a0a 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -312,7 +312,7 @@ func (gss *GameServerSpec) applySchedulingDefaults() { // devAddress is a specific IP address used for local Gameservers, for fleets "" is used // If a GameServer Spec is invalid there will be > 0 values in // the returned array -func (gss GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, bool) { +func (gss *GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, bool) { var causes []metav1.StatusCause if devAddress != "" { // verify that the value is a valid IP address. @@ -342,7 +342,7 @@ func (gss GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, boo } } else { // make sure a name is specified when there is multiple containers in the pod. - if len(gss.Container) == 0 && len(gss.Template.Spec.Containers) > 1 { + if gss.Container == "" && len(gss.Template.Spec.Containers) > 1 { causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Field: "container", @@ -389,7 +389,7 @@ func (gss GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, boo }) } } - objMetaCauses := validateObjectMeta(gss.Template.ObjectMeta) + objMetaCauses := validateObjectMeta(&gss.Template.ObjectMeta) if len(objMetaCauses) > 0 { causes = append(causes, objMetaCauses...) } diff --git a/pkg/apis/agones/v1/gameserverset.go b/pkg/apis/agones/v1/gameserverset.go index c79ad1332f..0abab73b12 100644 --- a/pkg/apis/agones/v1/gameserverset.go +++ b/pkg/apis/agones/v1/gameserverset.go @@ -79,9 +79,9 @@ type GameServerSetStatus struct { // ValidateUpdate validates when updates occur. The argument // is the new GameServerSet, being passed into the old GameServerSet -func (gsSet *GameServerSet) ValidateUpdate(new *GameServerSet) ([]metav1.StatusCause, bool) { - causes := validateName(new) - if !apiequality.Semantic.DeepEqual(gsSet.Spec.Template, new.Spec.Template) { +func (gsSet *GameServerSet) ValidateUpdate(newGSS *GameServerSet) ([]metav1.StatusCause, bool) { + causes := validateName(newGSS) + if !apiequality.Semantic.DeepEqual(gsSet.Spec.Template, newGSS.Spec.Template) { causes = append(causes, metav1.StatusCause{ Type: metav1.CauseTypeFieldValueInvalid, Field: "template", @@ -102,7 +102,7 @@ func (gsSet *GameServerSet) Validate() ([]metav1.StatusCause, bool) { causes = append(causes, gsCauses...) } - objMetaCauses := validateObjectMeta(gsSet.Spec.Template.ObjectMeta) + objMetaCauses := validateObjectMeta(&gsSet.Spec.Template.ObjectMeta) if len(objMetaCauses) > 0 { causes = append(causes, objMetaCauses...) } diff --git a/pkg/apis/agones/v1/register.go b/pkg/apis/agones/v1/register.go index deb7434290..40369d72d8 100644 --- a/pkg/apis/agones/v1/register.go +++ b/pkg/apis/agones/v1/register.go @@ -53,8 +53,8 @@ func init() { } // Adds the list of known types to api.Scheme. -func addKnownTypes(scheme *k8sruntime.Scheme) error { - scheme.AddKnownTypes(SchemeGroupVersion, +func addKnownTypes(apiScheme *k8sruntime.Scheme) error { + apiScheme.AddKnownTypes(SchemeGroupVersion, &GameServer{}, &GameServerList{}, &GameServerSet{}, @@ -62,6 +62,6 @@ func addKnownTypes(scheme *k8sruntime.Scheme) error { &Fleet{}, &FleetList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + metav1.AddToGroupVersion(apiScheme, SchemeGroupVersion) return nil } diff --git a/pkg/apis/allocation/v1/gameserverallocation.go b/pkg/apis/allocation/v1/gameserverallocation.go index fd470c9a8c..4aa60b444e 100644 --- a/pkg/apis/allocation/v1/gameserverallocation.go +++ b/pkg/apis/allocation/v1/gameserverallocation.go @@ -101,6 +101,7 @@ func (gsas *GameServerAllocationSpec) PreferredSelectors() ([]labels.Selector, e var err error for i, p := range gsas.Preferred { + p := p list[i], err = metav1.LabelSelectorAsSelector(&p) if err != nil { break diff --git a/pkg/apis/allocation/v1/register.go b/pkg/apis/allocation/v1/register.go index 0aa2ed3727..7f78c4bc7d 100644 --- a/pkg/apis/allocation/v1/register.go +++ b/pkg/apis/allocation/v1/register.go @@ -49,11 +49,11 @@ func init() { } // Adds the list of known types to api.Scheme. -func addKnownTypes(scheme *k8sruntime.Scheme) error { - scheme.AddKnownTypes(SchemeGroupVersion, +func addKnownTypes(apiScheme *k8sruntime.Scheme) error { + apiScheme.AddKnownTypes(SchemeGroupVersion, &GameServerAllocation{}, &GameServerAllocationList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + metav1.AddToGroupVersion(apiScheme, SchemeGroupVersion) return nil } diff --git a/pkg/apis/autoscaling/v1/register.go b/pkg/apis/autoscaling/v1/register.go index 90f27fe1dd..44103c5460 100644 --- a/pkg/apis/autoscaling/v1/register.go +++ b/pkg/apis/autoscaling/v1/register.go @@ -49,11 +49,11 @@ func init() { } // Adds the list of known types to api.Scheme. -func addKnownTypes(scheme *k8sruntime.Scheme) error { - scheme.AddKnownTypes(SchemeGroupVersion, +func addKnownTypes(apiScheme *k8sruntime.Scheme) error { + apiScheme.AddKnownTypes(SchemeGroupVersion, &FleetAutoscaler{}, &FleetAutoscalerList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + metav1.AddToGroupVersion(apiScheme, SchemeGroupVersion) return nil } diff --git a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go index 8426e66b28..14a262d5d4 100644 --- a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go +++ b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go @@ -175,11 +175,11 @@ func selectRandomWeighted(connections []*ClusterConnectionInfo, weights []int) * return nil } - rand := rand.Intn(sum) + randValue := rand.Intn(sum) sum = 0 for i, weight := range weights { sum += weight - if rand < sum { + if randValue < sum { return connections[i] } } diff --git a/pkg/apis/multicluster/v1alpha1/register.go b/pkg/apis/multicluster/v1alpha1/register.go index fe7ce11f5f..ca55dde2fe 100644 --- a/pkg/apis/multicluster/v1alpha1/register.go +++ b/pkg/apis/multicluster/v1alpha1/register.go @@ -49,11 +49,11 @@ func init() { } // Adds the list of known types to api.Scheme. -func addKnownTypes(scheme *k8sruntime.Scheme) error { - scheme.AddKnownTypes(SchemeGroupVersion, +func addKnownTypes(apiScheme *k8sruntime.Scheme) error { + apiScheme.AddKnownTypes(SchemeGroupVersion, &GameServerAllocationPolicy{}, &GameServerAllocationPolicyList{}, ) - metav1.AddToGroupVersion(scheme, SchemeGroupVersion) + metav1.AddToGroupVersion(apiScheme, SchemeGroupVersion) return nil } diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index 3d6d9b7f00..47b7b6a56f 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -386,16 +386,17 @@ func (c *Controller) deleteEmptyGameServerSets(fleet *agonesv1.Fleet, list []*ag // GameServerSets, and return the replica count for the active GameServerSet func (c *Controller) recreateDeployment(fleet *agonesv1.Fleet, rest []*agonesv1.GameServerSet) (int32, error) { for _, gsSet := range rest { - if gsSet.Spec.Replicas != 0 { - c.loggerForFleet(fleet).WithField("gameserverset", gsSet.ObjectMeta.Name).Info("applying recreate deployment: scaling to 0") - gsSetCopy := gsSet.DeepCopy() - gsSetCopy.Spec.Replicas = 0 - if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(gsSetCopy); err != nil { - return 0, errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) - } - c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", - "Scaling inactive GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) + if gsSet.Spec.Replicas == 0 { + continue + } + c.loggerForFleet(fleet).WithField("gameserverset", gsSet.ObjectMeta.Name).Info("applying recreate deployment: scaling to 0") + gsSetCopy := gsSet.DeepCopy() + gsSetCopy.Spec.Replicas = 0 + if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(gsSetCopy); err != nil { + return 0, errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) } + c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", + "Scaling inactive GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, gsSet.Spec.Replicas, gsSetCopy.Spec.Replicas) } return fleet.LowerBoundReplicas(fleet.Spec.Replicas - agonesv1.SumStatusAllocatedReplicas(rest)), nil diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index 627ee0aebe..cd32c9817d 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -340,7 +340,7 @@ func TestControllerRun(t *testing.T) { // test updating fleet fCopy := fleet.DeepCopy() - fCopy.Spec.Replicas = fCopy.Spec.Replicas + 10 + fCopy.Spec.Replicas += 10 fleetWatch.Modify(fCopy) assert.Equal(t, expected, f()) diff --git a/pkg/gameserverallocations/allocator.go b/pkg/gameserverallocations/allocator.go index 0886e8550d..a3d0740f00 100644 --- a/pkg/gameserverallocations/allocator.go +++ b/pkg/gameserverallocations/allocator.go @@ -238,11 +238,12 @@ func (c *Allocator) allocateFromLocalCluster(gsa *allocationv1.GameServerAllocat return nil, err } - if err == ErrNoGameServerReady { + switch err { + case ErrNoGameServerReady: gsa.Status.State = allocationv1.GameServerAllocationUnAllocated - } else if err == ErrConflictInGameServerSelection { + case ErrConflictInGameServerSelection: gsa.Status.State = allocationv1.GameServerAllocationContention - } else { + default: gsa.ObjectMeta.Name = gs.ObjectMeta.Name gsa.Status.State = allocationv1.GameServerAllocationAllocated gsa.Status.GameServerName = gs.ObjectMeta.Name @@ -532,7 +533,7 @@ func (c *Allocator) allocationUpdateWorkers(workerCount int, stop <-chan struct{ for { select { case res := <-updateQueue: - gs, err := c.readyGameServerCache.PatchGameServerMetadata(res.request.gsa.Spec.MetaPatch, *res.gs) + gs, err := c.readyGameServerCache.PatchGameServerMetadata(res.request.gsa.Spec.MetaPatch, res.gs) if err != nil { // since we could not allocate, we should put it back c.readyGameServerCache.AddToReadyGameServer(gs) diff --git a/pkg/gameserverallocations/cache_test.go b/pkg/gameserverallocations/cache_test.go index cfa52f01ee..c60d6faff1 100644 --- a/pkg/gameserverallocations/cache_test.go +++ b/pkg/gameserverallocations/cache_test.go @@ -44,11 +44,8 @@ func TestGameServerCacheEntry(t *testing.T) { count := 0 cache.Range(func(key string, gs *agonesv1.GameServer) bool { - if count++; count == 2 { - return false - } - - return true + count++ + return count != 2 }) assert.Equal(t, 2, count, "Should only process one item") diff --git a/pkg/gameserverallocations/controller_test.go b/pkg/gameserverallocations/controller_test.go index ee82dbf97d..6c5bfdd616 100644 --- a/pkg/gameserverallocations/controller_test.go +++ b/pkg/gameserverallocations/controller_test.go @@ -61,7 +61,7 @@ func TestControllerAllocator(t *testing.T) { stop := signals.NewStopChannel() t.Run("successful allocation", func(t *testing.T) { - f, _, gsList := defaultFixtures(3) + f, gsList := defaultFixtures(3) gsa := &allocationv1.GameServerAllocation{ Spec: allocationv1.GameServerAllocationSpec{ @@ -158,7 +158,7 @@ func TestControllerAllocator(t *testing.T) { func TestControllerAllocate(t *testing.T) { t.Parallel() - f, _, gsList := defaultFixtures(4) + f, gsList := defaultFixtures(4) c, m := newFakeController() n := metav1.Now() l := map[string]string{"mode": "deathmatch"} @@ -243,7 +243,7 @@ func TestControllerAllocatePriority(t *testing.T) { stop := signals.NewStopChannel() run := func(t *testing.T, name string, test func(t *testing.T, c *Controller, gas *allocationv1.GameServerAllocation)) { - f, _, gsList := defaultFixtures(4) + f, gsList := defaultFixtures(4) c, m := newFakeController() gsList[0].Status.NodeName = n1 @@ -346,7 +346,7 @@ func TestControllerRunLocalAllocations(t *testing.T) { t.Parallel() t.Run("no problems", func(t *testing.T) { - f, _, gsList := defaultFixtures(5) + f, gsList := defaultFixtures(5) gsList[0].Status.NodeName = "special" c, m := newFakeController() @@ -478,9 +478,9 @@ func TestAllocationApiResource(t *testing.T) { func TestControllerRunCacheSync(t *testing.T) { c, m := newFakeController() - watch := watch.NewFake() + gsWatch := watch.NewFake() - m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(watch, nil)) + m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(gsWatch, nil)) stop, cancel := agtesting.StartInformers(m, c.allocator.readyGameServerCache.gameServerSynced) defer cancel() @@ -511,52 +511,52 @@ func TestControllerRunCacheSync(t *testing.T) { } logrus.Info("adding ready game server") - watch.Add(gs.DeepCopy()) + gsWatch.Add(gs.DeepCopy()) assertCacheEntries(0) gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(1) // try again, should be no change gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(1) // now move it to Shutdown gs.Status.State = agonesv1.GameServerStateShutdown - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(0) // add back in ready gameserver gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(0) gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(1) // update with deletion timestamp n := metav1.Now() deletedCopy := gs.DeepCopy() deletedCopy.ObjectMeta.DeletionTimestamp = &n - watch.Modify(deletedCopy) + gsWatch.Modify(deletedCopy) assertCacheEntries(0) // add back in ready gameserver gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(0) gs.Status.State = agonesv1.GameServerStateReady - watch.Modify(gs.DeepCopy()) + gsWatch.Modify(gs.DeepCopy()) assertCacheEntries(1) // now actually delete it - watch.Delete(gs.DeepCopy()) + gsWatch.Delete(gs.DeepCopy()) assertCacheEntries(0) } @@ -573,7 +573,7 @@ func TestGetRandomlySelectedGS(t *testing.T) { }, } - _, _, gsList := defaultFixtures(10) + _, gsList := defaultFixtures(10) selectedGS := c.allocator.getRandomlySelectedGS(gsa, gsList) assert.NotNil(t, "selectedGS can't be nil", selectedGS) @@ -582,12 +582,12 @@ func TestGetRandomlySelectedGS(t *testing.T) { assert.NotEqual(t, expectedName, selectedGS.ObjectMeta.Name) } - _, _, gsList = defaultFixtures(5) + _, gsList = defaultFixtures(5) selectedGS = c.allocator.getRandomlySelectedGS(gsa, gsList) assert.NotNil(t, "selectedGS can't be nil", selectedGS) - _, _, gsList = defaultFixtures(1) + _, gsList = defaultFixtures(1) selectedGS = c.allocator.getRandomlySelectedGS(gsa, gsList) assert.NotNil(t, "selectedGS can't be nil", selectedGS) @@ -764,6 +764,8 @@ func TestControllerListSortedReadyGameServers(t *testing.T) { } for k, v := range fixtures { + k := k + v := v t.Run(k, func(t *testing.T) { c, m := newFakeController() @@ -1261,7 +1263,7 @@ func executeAllocation(gsa *allocationv1.GameServerAllocation, c *Controller) (* return nil, err } rec := httptest.NewRecorder() - if err = c.processAllocationRequest(rec, r, gsa.Namespace, stop); err != nil { + if err := c.processAllocationRequest(rec, r, gsa.Namespace, stop); err != nil { return nil, err } @@ -1271,7 +1273,7 @@ func executeAllocation(gsa *allocationv1.GameServerAllocation, c *Controller) (* } func addReactorForGameServer(m *agtesting.Mocks) string { - f, _, gsList := defaultFixtures(3) + f, gsList := defaultFixtures(3) gsWatch := watch.NewFake() m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(gsWatch, nil)) m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, k8sruntime.Object, error) { @@ -1301,7 +1303,7 @@ func createRequest(gsa *allocationv1.GameServerAllocation) (*http.Request, error return r, nil } -func defaultFixtures(gsLen int) (*agonesv1.Fleet, *agonesv1.GameServerSet, []agonesv1.GameServer) { +func defaultFixtures(gsLen int) (*agonesv1.Fleet, []agonesv1.GameServer) { f := &agonesv1.Fleet{ ObjectMeta: metav1.ObjectMeta{ Name: "fleet-1", @@ -1323,7 +1325,7 @@ func defaultFixtures(gsLen int) (*agonesv1.Fleet, *agonesv1.GameServerSet, []ago gs.Status.State = agonesv1.GameServerStateReady gsList = append(gsList, *gs) } - return f, gsSet, gsList + return f, gsList } // newFakeController returns a controller, backed by the fake Clientset diff --git a/pkg/gameserverallocations/ready_cache.go b/pkg/gameserverallocations/ready_cache.go index 36f84874dd..b68bb219a7 100644 --- a/pkg/gameserverallocations/ready_cache.go +++ b/pkg/gameserverallocations/ready_cache.go @@ -202,11 +202,11 @@ func (c *ReadyGameServerCache) ListSortedReadyGameServers() []*agonesv1.GameServ } // PatchGameServerMetadata patches the input gameserver with allocation meta patch and returns the updated gameserver -func (c *ReadyGameServerCache) PatchGameServerMetadata(fam allocationv1.MetaPatch, gs agonesv1.GameServer) (*agonesv1.GameServer, error) { - c.patchMetadata(&gs, fam) +func (c *ReadyGameServerCache) PatchGameServerMetadata(fam allocationv1.MetaPatch, gs *agonesv1.GameServer) (*agonesv1.GameServer, error) { + c.patchMetadata(gs, fam) gs.Status.State = agonesv1.GameServerStateAllocated - return c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(&gs) + return c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(gs) } // patch the labels and annotations of an allocated GameServer with metadata from a GameServerAllocation diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index 5a395e8202..dc2855991d 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -234,14 +234,14 @@ func (c *Controller) creationMutationHandler(review admv1beta1.AdmissionReview) return review, errors.Wrapf(err, "error creating patch for GameServer %s", gs.ObjectMeta.Name) } - json, err := json.Marshal(patch) + jsonPatch, err := json.Marshal(patch) if err != nil { return review, errors.Wrapf(err, "error creating json for patch for GameServer %s", gs.ObjectMeta.Name) } pt := admv1beta1.PatchTypeJSONPatch review.Response.PatchType = &pt - review.Response.Patch = json + review.Response.Patch = jsonPatch return review, nil } @@ -386,7 +386,7 @@ func (c *Controller) syncGameServer(key string) error { if gs, err = c.syncDevelopmentGameServer(gs); err != nil { return err } - if err = c.syncGameServerShutdownState(gs); err != nil { + if err := c.syncGameServerShutdownState(gs); err != nil { return err } @@ -654,7 +654,8 @@ func (c *Controller) addGameServerHealthCheck(gs *agonesv1.GameServer, pod *core } func (c *Controller) addSDKServerEnvVars(gs *agonesv1.GameServer, pod *corev1.Pod) { - for i, c := range pod.Spec.Containers { + for i := range pod.Spec.Containers { + c := &pod.Spec.Containers[i] if c.Name != sdkserverSidecarName { sdkEnvVars := sdkEnvironmentVariables(gs) if sdkEnvVars == nil { @@ -671,8 +672,9 @@ func (c *Controller) addSDKServerEnvVars(gs *agonesv1.GameServer, pod *corev1.Po env = append(env, e) } } - c.Env = append(env, sdkEnvVars...) - pod.Spec.Containers[i] = c + env = append(env, sdkEnvVars...) + c.Env = env + pod.Spec.Containers[i] = *c } } } @@ -823,10 +825,10 @@ func (c *Controller) syncGameServerShutdownState(gs *agonesv1.GameServer) error // moveToErrorState moves the GameServer to the error state func (c *Controller) moveToErrorState(gs *agonesv1.GameServer, msg string) (*agonesv1.GameServer, error) { - copy := gs.DeepCopy() - copy.Status.State = agonesv1.GameServerStateError + gsCopy := gs.DeepCopy() + gsCopy.Status.State = agonesv1.GameServerStateError - gs, err := c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(copy) + gs, err := c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(gsCopy) if err != nil { return gs, errors.Wrapf(err, "error moving GameServer %s to Error State", gs.ObjectMeta.Name) } diff --git a/pkg/gameservers/pernodecounter_test.go b/pkg/gameservers/pernodecounter_test.go index 3d9cda1e88..fbba4536be 100644 --- a/pkg/gameservers/pernodecounter_test.go +++ b/pkg/gameservers/pernodecounter_test.go @@ -39,8 +39,8 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { pnc, m := newFakePerNodeCounter() - watch := watch.NewFake() - m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(watch, nil)) + fakeWatch := watch.NewFake() + m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(fakeWatch, nil)) hasSynced := m.AgonesInformerFactory.Agones().V1().GameServers().Informer().HasSynced stop, cancel := agtesting.StartInformers(m) @@ -51,15 +51,17 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { gs := &agonesv1.GameServer{ ObjectMeta: metav1.ObjectMeta{Name: "gs1", Namespace: defaultNs}, Status: agonesv1.GameServerStatus{ - State: agonesv1.GameServerStateStarting, NodeName: name1}} + State: agonesv1.GameServerStateStarting, NodeName: name1, + }, + } - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) assert.Empty(t, pnc.Counts()) gs.Status.State = agonesv1.GameServerStateReady - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) counts := pnc.Counts() @@ -68,7 +70,7 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { assert.Equal(t, int64(0), counts[name1].Allocated) gs.Status.State = agonesv1.GameServerStateAllocated - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) counts = pnc.Counts() @@ -77,7 +79,7 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { assert.Equal(t, int64(1), counts[name1].Allocated) gs.Status.State = agonesv1.GameServerStateShutdown - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) counts = pnc.Counts() @@ -89,7 +91,7 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { gs.Status.State = agonesv1.GameServerStateReady gs.Status.NodeName = name2 - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) counts = pnc.Counts() @@ -104,7 +106,7 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) { gs.Status.State = agonesv1.GameServerStateAllocated gs.Status.NodeName = name2 - watch.Add(gs.DeepCopy()) + fakeWatch.Add(gs.DeepCopy()) cache.WaitForCacheSync(stop, hasSynced) counts = pnc.Counts() diff --git a/pkg/gameservers/portallocator.go b/pkg/gameservers/portallocator.go index 0ba7b482af..4f2ad76318 100644 --- a/pkg/gameservers/portallocator.go +++ b/pkg/gameservers/portallocator.go @@ -147,16 +147,17 @@ func (pa *PortAllocator) Allocate(gs *agonesv1.GameServer) *agonesv1.GameServer pa.gameServerRegistry[gs.ObjectMeta.UID] = true for i, p := range gs.Spec.Ports { - if p.PortPolicy == agonesv1.Dynamic || p.PortPolicy == agonesv1.Passthrough { - // pop off allocation - var a pn - a, allocations = allocations[0], allocations[1:] - a.pa[a.port] = true - gs.Spec.Ports[i].HostPort = a.port - - if p.PortPolicy == agonesv1.Passthrough { - gs.Spec.Ports[i].ContainerPort = a.port - } + if p.PortPolicy != agonesv1.Dynamic && p.PortPolicy != agonesv1.Passthrough { + continue + } + // pop off allocation + var a pn + a, allocations = allocations[0], allocations[1:] + a.pa[a.port] = true + gs.Spec.Ports[i].HostPort = a.port + + if p.PortPolicy == agonesv1.Passthrough { + gs.Spec.Ports[i].ContainerPort = a.port } } @@ -271,17 +272,18 @@ func (pa *PortAllocator) registerExistingGameServerPorts(gameservers []*agonesv1 for _, gs := range gameservers { for _, p := range gs.Spec.Ports { - if p.PortPolicy == agonesv1.Dynamic || p.PortPolicy == agonesv1.Passthrough { - gsRegistry[gs.ObjectMeta.UID] = true - - // if the node doesn't exist, it's likely unscheduled - _, ok := nodePortAllocation[gs.Status.NodeName] - if gs.Status.NodeName != "" && ok { - nodePortAllocation[gs.Status.NodeName][p.HostPort] = true - nodePortCount[gs.Status.NodeName]++ - } else if p.HostPort != 0 { - nonReadyNodesPorts = append(nonReadyNodesPorts, p.HostPort) - } + if p.PortPolicy != agonesv1.Dynamic && p.PortPolicy != agonesv1.Passthrough { + continue + } + gsRegistry[gs.ObjectMeta.UID] = true + + // if the node doesn't exist, it's likely unscheduled + _, ok := nodePortAllocation[gs.Status.NodeName] + if gs.Status.NodeName != "" && ok { + nodePortAllocation[gs.Status.NodeName][p.HostPort] = true + nodePortCount[gs.Status.NodeName]++ + } else if p.HostPort != 0 { + nonReadyNodesPorts = append(nonReadyNodesPorts, p.HostPort) } } } diff --git a/pkg/gameservers/portallocator_test.go b/pkg/gameservers/portallocator_test.go index 56b7693bfc..0d69832afb 100644 --- a/pkg/gameservers/portallocator_test.go +++ b/pkg/gameservers/portallocator_test.go @@ -71,39 +71,39 @@ func TestPortAllocatorAllocate(t *testing.T) { assert.Equal(t, 2, countTotalAllocatedPorts(pa)) // double port, dynamic - copy := fixture.DeepCopy() - copy.Spec.Ports = append(copy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}) - assert.Len(t, copy.Spec.Ports, 2) - pa.Allocate(copy.DeepCopy()) + gsCopy := fixture.DeepCopy() + gsCopy.Spec.Ports = append(gsCopy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}) + assert.Len(t, gsCopy.Spec.Ports, 2) + pa.Allocate(gsCopy.DeepCopy()) assert.Nil(t, err) assert.Equal(t, 4, countTotalAllocatedPorts(pa)) // three ports, dynamic - copy = copy.DeepCopy() - copy.Spec.Ports = append(copy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}) - assert.Len(t, copy.Spec.Ports, 3) - pa.Allocate(copy) + gsCopy = gsCopy.DeepCopy() + gsCopy.Spec.Ports = append(gsCopy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}) + assert.Len(t, gsCopy.Spec.Ports, 3) + pa.Allocate(gsCopy) assert.Nil(t, err) assert.Equal(t, 7, countTotalAllocatedPorts(pa)) // 4 ports, 1 static, rest dynamic - copy = copy.DeepCopy() + gsCopy = gsCopy.DeepCopy() expected := int32(9999) - copy.Spec.Ports = append(copy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, HostPort: expected, PortPolicy: agonesv1.Static}) - assert.Len(t, copy.Spec.Ports, 4) - pa.Allocate(copy) + gsCopy.Spec.Ports = append(gsCopy.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, HostPort: expected, PortPolicy: agonesv1.Static}) + assert.Len(t, gsCopy.Spec.Ports, 4) + pa.Allocate(gsCopy) assert.Nil(t, err) assert.Equal(t, 10, countTotalAllocatedPorts(pa)) - assert.Equal(t, agonesv1.Static, copy.Spec.Ports[3].PortPolicy) - assert.Equal(t, expected, copy.Spec.Ports[3].HostPort) + assert.Equal(t, agonesv1.Static, gsCopy.Spec.Ports[3].PortPolicy) + assert.Equal(t, expected, gsCopy.Spec.Ports[3].HostPort) // single port, passthrough - copy = fixture.DeepCopy() - copy.Spec.Ports[0] = agonesv1.GameServerPort{Name: "passthrough", PortPolicy: agonesv1.Passthrough} - assert.Len(t, copy.Spec.Ports, 1) - pa.Allocate(copy) - assert.NotEmpty(t, copy.Spec.Ports[0].HostPort) - assert.Equal(t, copy.Spec.Ports[0].HostPort, copy.Spec.Ports[0].ContainerPort) + gsCopy = fixture.DeepCopy() + gsCopy.Spec.Ports[0] = agonesv1.GameServerPort{Name: "passthrough", PortPolicy: agonesv1.Passthrough} + assert.Len(t, gsCopy.Spec.Ports, 1) + pa.Allocate(gsCopy) + assert.NotEmpty(t, gsCopy.Spec.Ports[0].HostPort) + assert.Equal(t, gsCopy.Spec.Ports[0].HostPort, gsCopy.Spec.Ports[0].ContainerPort) assert.Nil(t, err) assert.Equal(t, 11, countTotalAllocatedPorts(pa)) }) @@ -153,9 +153,10 @@ func TestPortAllocatorAllocate(t *testing.T) { defer cancel() morePortFixture := fixture.DeepCopy() - morePortFixture.Spec.Ports = append(morePortFixture.Spec.Ports, agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}) - morePortFixture.Spec.Ports = append(morePortFixture.Spec.Ports, agonesv1.GameServerPort{Name: "static", ContainerPort: 6666, PortPolicy: agonesv1.Static, HostPort: 9999}) - morePortFixture.Spec.Ports = append(morePortFixture.Spec.Ports, agonesv1.GameServerPort{Name: "passthrough", PortPolicy: agonesv1.Passthrough}) + morePortFixture.Spec.Ports = append(morePortFixture.Spec.Ports, + agonesv1.GameServerPort{Name: "another", ContainerPort: 6666, PortPolicy: agonesv1.Dynamic}, + agonesv1.GameServerPort{Name: "static", ContainerPort: 6666, PortPolicy: agonesv1.Static, HostPort: 9999}, + agonesv1.GameServerPort{Name: "passthrough", PortPolicy: agonesv1.Passthrough}) assert.Len(t, morePortFixture.Spec.Ports, 4) @@ -171,9 +172,9 @@ func TestPortAllocatorAllocate(t *testing.T) { for x := 0; x < 2; x++ { // ports between 10 and 20, but there are 2 ports for i := 10; i <= 14; i++ { - copy := morePortFixture.DeepCopy() - copy.ObjectMeta.UID = types.UID(strconv.Itoa(x) + ":" + strconv.Itoa(i)) - gs := pa.Allocate(copy) + gsCopy := morePortFixture.DeepCopy() + gsCopy.ObjectMeta.UID = types.UID(strconv.Itoa(x) + ":" + strconv.Itoa(i)) + gs := pa.Allocate(gsCopy) // Dynamic assert.NotEmpty(t, gs.Spec.Ports[0].HostPort) @@ -184,7 +185,7 @@ func TestPortAllocatorAllocate(t *testing.T) { assert.NotEmpty(t, passThrough.HostPort) assert.Equal(t, passThrough.HostPort, passThrough.ContainerPort) - logrus.WithField("uid", copy.ObjectMeta.UID).WithField("ports", gs.Spec.Ports).WithError(err).Info("Allocated Port") + logrus.WithField("uid", gsCopy.ObjectMeta.UID).WithField("ports", gs.Spec.Ports).WithError(err).Info("Allocated Port") assert.Nil(t, err) for _, p := range gs.Spec.Ports { if p.PortPolicy == agonesv1.Dynamic || p.PortPolicy == agonesv1.Passthrough { diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index 019ebef607..70f1927cd1 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -315,7 +315,7 @@ func (c *Controller) syncGameServerSet(key string) error { for _, gs := range list { key := "gsCount" + string(gs.Status.State) if gs.ObjectMeta.DeletionTimestamp != nil { - key = key + "Deleted" + key += "Deleted" } v, ok := fields[key] if !ok { diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index e842e9c239..44f9fc381a 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -589,9 +589,9 @@ func TestControllerUpdateValidationHandler(t *testing.T) { assert.Nil(t, err) t.Run("valid gameserverset update", func(t *testing.T) { - new := fixture.DeepCopy() - new.Spec.Replicas = 10 - newRaw, err := json.Marshal(new) + newGSS := fixture.DeepCopy() + newGSS.Spec.Replicas = 10 + newRaw, err := json.Marshal(newGSS) assert.Nil(t, err) review := admv1beta1.AdmissionReview{ @@ -614,13 +614,13 @@ func TestControllerUpdateValidationHandler(t *testing.T) { }) t.Run("invalid gameserverset update", func(t *testing.T) { - new := fixture.DeepCopy() - new.Spec.Template = agonesv1.GameServerTemplateSpec{ + newGSS := fixture.DeepCopy() + newGSS.Spec.Template = agonesv1.GameServerTemplateSpec{ Spec: agonesv1.GameServerSpec{ Ports: []agonesv1.GameServerPort{{PortPolicy: agonesv1.Static}}, }, } - newRaw, err := json.Marshal(new) + newRaw, err := json.Marshal(newGSS) assert.Nil(t, err) assert.NotEqual(t, string(raw), string(newRaw)) diff --git a/pkg/metrics/controller.go b/pkg/metrics/controller.go index 944ea37d65..7f0358cf9d 100644 --- a/pkg/metrics/controller.go +++ b/pkg/metrics/controller.go @@ -96,8 +96,8 @@ func NewController( fInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.recordFleetChanges, - UpdateFunc: func(old, new interface{}) { - c.recordFleetChanges(new) + UpdateFunc: func(old, next interface{}) { + c.recordFleetChanges(next) }, DeleteFunc: c.recordFleetDeletion, }) @@ -117,9 +117,9 @@ func NewController( return c } -func (c *Controller) recordFleetAutoScalerChanges(old, new interface{}) { +func (c *Controller) recordFleetAutoScalerChanges(old, next interface{}) { - fas, ok := new.(*autoscalingv1.FleetAutoscaler) + fas, ok := next.(*autoscalingv1.FleetAutoscaler) if !ok { return } @@ -246,8 +246,8 @@ func (c *Controller) recordFleetReplicas(fleetName string, total, allocated, rea // per second. // Addition to the cache are not handled, otherwise resync would make metrics inaccurate by doubling // current gameservers states. -func (c *Controller) recordGameServerStatusChanges(old, new interface{}) { - newGs, ok := new.(*agonesv1.GameServer) +func (c *Controller) recordGameServerStatusChanges(old, next interface{}) { + newGs, ok := next.(*agonesv1.GameServer) if !ok { return } diff --git a/pkg/metrics/exporter.go b/pkg/metrics/exporter.go index 3a6d50784d..4a4cc323f6 100644 --- a/pkg/metrics/exporter.go +++ b/pkg/metrics/exporter.go @@ -80,16 +80,16 @@ func RegisterStackdriverExporter(projectID string, defaultLabels string) (*stack // SetReportingPeriod set appropriate reporting period which depends on exporters // we are going to use -func SetReportingPeriod(prometheus, stackdriver bool) { +func SetReportingPeriod(forPrometheus, forStackdriver bool) { // if we're using only prometheus we can report faster as we're only exposing metrics in memory reportingPeriod := 15 * time.Second - if stackdriver { + if forStackdriver { // There is a limitation on Stackdriver that reporting should // be equal or more than 1 minute reportingPeriod = 60 * time.Second } - if stackdriver || prometheus { + if forStackdriver || forPrometheus { view.SetReportingPeriod(reportingPeriod) } } diff --git a/pkg/sdkserver/localsdk.go b/pkg/sdkserver/localsdk.go index 53ca49f732..edcd548ab3 100644 --- a/pkg/sdkserver/localsdk.go +++ b/pkg/sdkserver/localsdk.go @@ -103,16 +103,17 @@ func NewLocalSDKServer(filePath string) (*LocalSDKServer, error) { go func() { for event := range watcher.Events { - if event.Op == fsnotify.Write { - logrus.WithField("event", event).Info("File has been changed!") - err := l.setGameServerFromFilePath(filePath) - if err != nil { - logrus.WithError(err).Error("error setting GameServer from file") - continue - } - logrus.Info("Sending watched GameServer!") - l.update <- struct{}{} + if event.Op != fsnotify.Write { + continue } + logrus.WithField("event", event).Info("File has been changed!") + err := l.setGameServerFromFilePath(filePath) + if err != nil { + logrus.WithError(err).Error("error setting GameServer from file") + continue + } + logrus.Info("Sending watched GameServer!") + l.update <- struct{}{} } }() @@ -167,11 +168,12 @@ func (l *LocalSDKServer) recordRequest(request string) { func (l *LocalSDKServer) recordRequestWithValue(request string, value string, objMetaField string) { if l.testMode { fieldVal := "" - if objMetaField == "CreationTimestamp" { + switch objMetaField { + case "CreationTimestamp": fieldVal = strconv.FormatInt(l.gs.ObjectMeta.CreationTimestamp, 10) - } else if objMetaField == "UID" { + case "UID": fieldVal = l.gs.ObjectMeta.Uid - } else { + default: fmt.Printf("Error: Unexpected Field to compare") } diff --git a/pkg/util/apiserver/apiserver.go b/pkg/util/apiserver/apiserver.go index 5a5fe53332..253861a56f 100644 --- a/pkg/util/apiserver/apiserver.go +++ b/pkg/util/apiserver/apiserver.go @@ -108,10 +108,10 @@ func NewAPIServer(mux *http.ServeMux) *APIServer { // e.g. http://localhost:8001/apis/scheduling.k8s.io/v1beta1 // as well as registering a CRDHandler that all http requests for the given APIResource are routed to func (as *APIServer) AddAPIResource(groupVersion string, resource metav1.APIResource, handler CRDHandler) { - list, ok := as.resourceList[groupVersion] + _, ok := as.resourceList[groupVersion] if !ok { // discovery handler - list = &metav1.APIResourceList{GroupVersion: groupVersion, APIResources: []metav1.APIResource{}} + list := &metav1.APIResourceList{GroupVersion: groupVersion, APIResources: []metav1.APIResource{}} as.resourceList[groupVersion] = list pattern := fmt.Sprintf("/apis/%s", groupVersion) as.addSerializedHandler(pattern, list) @@ -125,7 +125,7 @@ func (as *APIServer) AddAPIResource(groupVersion string, resource metav1.APIReso } // discovery resource - list.APIResources = append(as.resourceList[groupVersion].APIResources, resource) + as.resourceList[groupVersion].APIResources = append(as.resourceList[groupVersion].APIResources, resource) // add specific crd resource handler key := fmt.Sprintf("%s/%s", groupVersion, resource.Name) @@ -149,7 +149,7 @@ func (as *APIServer) resourceHandler(gv string) https.ErrorHandlerFunc { return nil } - if err = delegate(w, r, namespace); err != nil { + if err := delegate(w, r, namespace); err != nil { return err } @@ -192,7 +192,7 @@ func AcceptedSerializer(r *http.Request, codecs serializer.CodecFactory) (k8srun } header := r.Header.Get(AcceptHeader) accept := goautoneg.Negotiate(header, alternatives) - if len(accept) == 0 { + if accept == "" { accept = k8sruntime.ContentTypeJSON } info, ok := k8sruntime.SerializerInfoForMediaType(mediaTypes, accept) diff --git a/pkg/util/crd/crd.go b/pkg/util/crd/crd.go index c6c6cfe507..6ac86b5b18 100644 --- a/pkg/util/crd/crd.go +++ b/pkg/util/crd/crd.go @@ -36,8 +36,7 @@ func WaitForEstablishedCRD(crdGetter extv1beta1.CustomResourceDefinitionInterfac } for _, cond := range crd.Status.Conditions { - switch cond.Type { - case apiv1beta1.Established: + if cond.Type == apiv1beta1.Established { if cond.Status == apiv1beta1.ConditionTrue { logger.WithField("crd", crd.ObjectMeta.Name).Info("custom resource definition established") return true, err diff --git a/pkg/util/runtime/runtime_test.go b/pkg/util/runtime/runtime_test.go index 81ae813051..f171208d90 100644 --- a/pkg/util/runtime/runtime_test.go +++ b/pkg/util/runtime/runtime_test.go @@ -37,7 +37,7 @@ func TestHandleError(t *testing.T) { assert.Nil(t, result, "No Errors for now") err := fmt.Errorf("test") - //test nil logger + // test nil logger logger := NewLoggerWithSource("test") HandleError(logger.WithError(err), err) if result != err { diff --git a/sdks/go/sdk_test.go b/sdks/go/sdk_test.go index e21699e48e..5602faac11 100644 --- a/sdks/go/sdk_test.go +++ b/sdks/go/sdk_test.go @@ -35,7 +35,7 @@ func TestSDK(t *testing.T) { health: sm.hm, } - //gate + // gate assert.False(t, sm.ready) assert.False(t, sm.shutdown) assert.False(t, sm.hm.healthy) diff --git a/site/handler.go b/site/handler.go index 548b2bfbf9..7a3ab88dcc 100644 --- a/site/handler.go +++ b/site/handler.go @@ -131,7 +131,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { func (h *handler) serveIndex(w http.ResponseWriter, r *http.Request) { // Just redirect to the first one // just commenting out, in case we want to soft launch - //http.Redirect(w, r, h.paths[0].repo, http.StatusTemporaryRedirect) + // http.Redirect(w, r, h.paths[0].repo, http.StatusTemporaryRedirect) http.Redirect(w, r, "/site/", http.StatusTemporaryRedirect) } diff --git a/test/e2e/allocator_test.go b/test/e2e/allocator_test.go index c6219ad96c..5d53703fe2 100644 --- a/test/e2e/allocator_test.go +++ b/test/e2e/allocator_test.go @@ -297,7 +297,8 @@ func restartAllocator(t *testing.T) { if err != nil { t.Fatalf("listing pods failed: %s", err) } - for _, pod := range pods.Items { + for i := range pods.Items { + pod := &pods.Items[i] if !strings.HasPrefix(pod.Name, allocatorServiceName) { continue } diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index 2771867771..5b87ba583c 100644 --- a/test/e2e/fleet_test.go +++ b/test/e2e/fleet_test.go @@ -385,7 +385,7 @@ func TestFleetUpdates(t *testing.T) { defer client.Fleets(defaultNs).Delete(flt.ObjectMeta.Name, nil) // nolint:errcheck } - err = framework.WaitForFleetGameServersCondition(flt, func(gs agonesv1.GameServer) bool { + err = framework.WaitForFleetGameServersCondition(flt, func(gs *agonesv1.GameServer) bool { return gs.ObjectMeta.Annotations[key] == red }) assert.Nil(t, err) @@ -408,7 +408,7 @@ func TestFleetUpdates(t *testing.T) { }) assert.Nil(t, err) - err = framework.WaitForFleetGameServersCondition(flt, func(gs agonesv1.GameServer) bool { + err = framework.WaitForFleetGameServersCondition(flt, func(gs *agonesv1.GameServer) bool { return gs.ObjectMeta.Annotations[key] == green }) assert.Nil(t, err) @@ -429,7 +429,7 @@ func TestUpdateGameServerConfigurationInFleet(t *testing.T) { PortPolicy: agonesv1.Dynamic, Protocol: corev1.ProtocolUDP, }} - flt := fleetWithGameServerSpec(gsSpec, defaultNs) + flt := fleetWithGameServerSpec(&gsSpec, defaultNs) flt, err := client.Fleets(defaultNs).Create(flt) assert.Nil(t, err, "could not create fleet") defer client.Fleets(defaultNs).Delete(flt.ObjectMeta.Name, nil) // nolint:errcheck @@ -463,7 +463,7 @@ func TestUpdateGameServerConfigurationInFleet(t *testing.T) { _, err = framework.AgonesClient.AgonesV1().Fleets(defaultNs).Update(fltCopy) assert.Nil(t, err, "could not update fleet") - err = framework.WaitForFleetGameServersCondition(flt, func(gs agonesv1.GameServer) bool { + err = framework.WaitForFleetGameServersCondition(flt, func(gs *agonesv1.GameServer) bool { containerPort := gs.Spec.Ports[0].ContainerPort return (gs.Name == gsa.Status.GameServerName && containerPort == oldPort) || (gs.Name != gsa.Status.GameServerName && containerPort == newPort) @@ -831,7 +831,7 @@ func TestScaleUpAndDownInParallelStressTest(t *testing.T) { framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(defaultReplicas)) } } - errors := make(chan error) + errorsChan := make(chan error) var wg sync.WaitGroup finished := make(chan bool, 1) @@ -849,7 +849,7 @@ func TestScaleUpAndDownInParallelStressTest(t *testing.T) { duration, err := scaleAndWait(t, flt, 0) if err != nil { fmt.Println(err) - errors <- err + errorsChan <- err return } scaleDownStats.ReportDuration(duration, nil) @@ -861,14 +861,14 @@ func TestScaleUpAndDownInParallelStressTest(t *testing.T) { duration, err := scaleAndWait(t, flt, fleetSize) if err != nil { fmt.Println(err) - errors <- err + errorsChan <- err return } scaleUpStats.ReportDuration(duration, nil) duration, err = scaleAndWait(t, flt, 0) if err != nil { fmt.Println(err) - errors <- err + errorsChan <- err return } scaleDownStats.ReportDuration(duration, nil) @@ -882,7 +882,7 @@ func TestScaleUpAndDownInParallelStressTest(t *testing.T) { select { case <-finished: - case err := <-errors: + case err := <-errorsChan: t.Fatalf("Error in waiting for a fleet to scale: %s", err) } fmt.Println("We are Done") @@ -1024,6 +1024,7 @@ func TestFleetRecreateGameServers(t *testing.T) { podClient := framework.KubeClient.CoreV1().Pods(defaultNs) for _, gs := range list.Items { + gs := gs pod, err := podClient.Get(gs.ObjectMeta.Name, metav1.GetOptions{}) assert.NoError(t, err) @@ -1035,6 +1036,7 @@ func TestFleetRecreateGameServers(t *testing.T) { }}, "gameserver shutdown": {f: func(t *testing.T, list *agonesv1.GameServerList) { for _, gs := range list.Items { + gs := gs var reply string reply, err := e2e.SendGameServerUDP(&gs, "EXIT") if err != nil { @@ -1046,6 +1048,7 @@ func TestFleetRecreateGameServers(t *testing.T) { }}, "gameserver unhealthy": {f: func(t *testing.T, list *agonesv1.GameServerList) { for _, gs := range list.Items { + gs := gs var reply string reply, err := e2e.SendGameServerUDP(&gs, "UNHEALTHY") if err != nil { @@ -1109,7 +1112,8 @@ func listGameServers(flt *agonesv1.Fleet, getter typedagonesv1.GameServersGetter // Counts the number of gameservers with the specified scheduling strategy in a fleet func countFleetScheduling(gsList []agonesv1.GameServer, scheduling apis.SchedulingStrategy) int { count := 0 - for _, gs := range gsList { + for i := range gsList { + gs := &gsList[i] if gs.Spec.Scheduling == scheduling { count++ } @@ -1190,17 +1194,17 @@ func scaleFleetSubresource(t *testing.T, f *agonesv1.Fleet, scale int32) *agones // defaultFleet returns a default fleet configuration func defaultFleet(namespace string) *agonesv1.Fleet { gs := defaultGameServer(namespace) - return fleetWithGameServerSpec(gs.Spec, namespace) + return fleetWithGameServerSpec(&gs.Spec, namespace) } // fleetWithGameServerSpec returns a fleet with specified gameserver spec -func fleetWithGameServerSpec(gsSpec agonesv1.GameServerSpec, namespace string) *agonesv1.Fleet { +func fleetWithGameServerSpec(gsSpec *agonesv1.GameServerSpec, namespace string) *agonesv1.Fleet { return &agonesv1.Fleet{ ObjectMeta: metav1.ObjectMeta{GenerateName: "simple-fleet-", Namespace: namespace}, Spec: agonesv1.FleetSpec{ Replicas: replicasCount, Template: agonesv1.GameServerTemplateSpec{ - Spec: gsSpec, + Spec: *gsSpec, }, }, } diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 3d9592e9bd..e217a723cc 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -190,7 +190,8 @@ func (f *Framework) ListGameServersFromFleet(flt *agonesv1.Fleet) ([]agonesv1.Ga return results, err } - for _, gsSet := range gsSetList.Items { + for i := range gsSetList.Items { + gsSet := &gsSetList.Items[i] opts := metav1.ListOptions{LabelSelector: labels.Set{agonesv1.GameServerSetGameServerLabel: gsSet.ObjectMeta.Name}.String()} gsList, err := f.AgonesClient.AgonesV1().GameServers(flt.ObjectMeta.Namespace).List(opts) if err != nil { @@ -214,10 +215,11 @@ func FleetReadyCount(amount int32) func(fleet *agonesv1.Fleet) bool { // WaitForFleetGameServersCondition waits for all GameServers for a given fleet to match // a condition specified by a callback. func (f *Framework) WaitForFleetGameServersCondition(flt *agonesv1.Fleet, - cond func(server agonesv1.GameServer) bool) error { + cond func(server *agonesv1.GameServer) bool) error { return f.WaitForFleetGameServerListCondition(flt, func(servers []agonesv1.GameServer) bool { - for _, gs := range servers { + for i := range servers { + gs := &servers[i] if !cond(gs) { return false } @@ -255,7 +257,7 @@ func (f *Framework) NewStatsCollector(name string) *StatsCollector { func (f *Framework) CleanUp(ns string) error { logrus.Info("Cleaning up now.") defer logrus.Info("Finished cleanup.") - agonesv1 := f.AgonesClient.AgonesV1() + agonesV1 := f.AgonesClient.AgonesV1() deleteOptions := &metav1.DeleteOptions{} listOptions := metav1.ListOptions{} @@ -268,13 +270,14 @@ func (f *Framework) CleanUp(ns string) error { return err } - for _, p := range podList.Items { - if err = pods.Delete(p.ObjectMeta.Name, deleteOptions); err != nil { + for i := range podList.Items { + p := &podList.Items[i] + if err := pods.Delete(p.ObjectMeta.Name, deleteOptions); err != nil { return err } } - err = agonesv1.Fleets(ns).DeleteCollection(deleteOptions, listOptions) + err = agonesV1.Fleets(ns).DeleteCollection(deleteOptions, listOptions) if err != nil { return err } @@ -284,7 +287,7 @@ func (f *Framework) CleanUp(ns string) error { return err } - return agonesv1.GameServers(ns). + return agonesV1.GameServers(ns). DeleteCollection(deleteOptions, listOptions) } @@ -411,7 +414,8 @@ func (f *Framework) DeleteNamespace(t *testing.T, namespace string) { if err != nil { t.Fatalf("listing pods in namespace %s failed: %s", namespace, err) } - for _, pod := range pods.Items { + for i := range pods.Items { + pod := &pods.Items[i] if len(pod.Finalizers) > 0 { pod.Finalizers = nil payload := []patchRemoveNoValue{{