Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions pkg/gameserversets/allocation_overflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (
"agones.dev/agones/pkg/client/informers/externalversions"
listerv1 "agones.dev/agones/pkg/client/listers/agones/v1"
"agones.dev/agones/pkg/gameservers"
"agones.dev/agones/pkg/util/errors"
"agones.dev/agones/pkg/util/logfields"
"agones.dev/agones/pkg/util/runtime"
"agones.dev/agones/pkg/util/workerqueue"
"github.com/heptiolabs/healthcheck"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -48,6 +48,7 @@ type AllocationOverflowController struct {
gameServerSetSynced cache.InformerSynced
gameServerSetLister listerv1.GameServerSetLister
workerqueue *workerqueue.WorkerQueue
errs *errors.Errors
}

// NewAllocatorOverflowController returns a new AllocationOverflowController
Expand All @@ -70,6 +71,7 @@ func NewAllocatorOverflowController(
}

c.baseLogger = runtime.NewLoggerWithType(c)
c.errs = errors.FromStruct(c)
c.baseLogger.Debug("Created!")
c.workerqueue = workerqueue.NewWorkerQueueWithRateLimiter(c.syncGameServerSet, c.baseLogger, logfields.GameServerSetKey, agones.GroupName+".GameServerSetController", workerqueue.FastRateLimiter(3*time.Second))
health.AddLivenessCheck("gameserverset-allocationoverflow-workerqueue", c.workerqueue.Healthy)
Expand Down Expand Up @@ -97,7 +99,7 @@ func NewAllocatorOverflowController(
func (c *AllocationOverflowController) Run(ctx context.Context) error {
c.baseLogger.Debug("Wait for cache sync")
if !cache.WaitForCacheSync(ctx.Done(), c.gameServerSynced, c.gameServerSetSynced) {
return errors.New("failed to wait for caches to sync")
return c.errs.New("failed to wait for caches to sync")
}

c.workerqueue.Run(ctx, 1)
Expand All @@ -111,7 +113,7 @@ func (c *AllocationOverflowController) syncGameServerSet(ctx context.Context, ke
namespace, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
// don't return an error, as we don't want this retried
runtime.HandleError(loggerForGameServerSetKey(c.baseLogger, key), errors.Wrapf(err, "invalid resource key"))
runtime.HandleError(loggerForGameServerSetKey(c.baseLogger, key), c.errs.Wrapf(err, "invalid resource key"))
return nil
}

Expand All @@ -121,7 +123,7 @@ func (c *AllocationOverflowController) syncGameServerSet(ctx context.Context, ke
loggerForGameServerSetKey(c.baseLogger, key).Debug("GameServerSet is no longer available for syncing")
return nil
}
return errors.Wrapf(err, "error retrieving GameServerSet %s from namespace %s", name, namespace)
return c.errs.Wrapf(err, "error retrieving GameServerSet %s from namespace %s", name, namespace)
}

// just in case something changed, double check to avoid panics and/or sending work to the K8s API that we don't
Expand Down Expand Up @@ -154,7 +156,7 @@ func (c *AllocationOverflowController) syncGameServerSet(ctx context.Context, ke
gsSet.Spec.AllocationOverflow.Apply(gsCopy)

if _, err := c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, opts); err != nil {
return errors.Wrapf(err, "error updating GameServer %s with overflow labels and/or annotations", gs.ObjectMeta.Name)
return c.errs.Wrapf(err, "error updating GameServer %s with overflow labels and/or annotations", gs.ObjectMeta.Name)
}
}

Expand Down
32 changes: 18 additions & 14 deletions pkg/gameserversets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ import (
listerv1 "agones.dev/agones/pkg/client/listers/agones/v1"
"agones.dev/agones/pkg/gameservers"
"agones.dev/agones/pkg/util/crd"
"agones.dev/agones/pkg/util/errors"
"agones.dev/agones/pkg/util/logfields"
"agones.dev/agones/pkg/util/runtime"
"agones.dev/agones/pkg/util/webhooks"
"agones.dev/agones/pkg/util/workerqueue"
"github.com/google/go-cmp/cmp"
"github.com/heptiolabs/healthcheck"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"go.opencensus.io/tag"
admissionv1 "k8s.io/api/admission/v1"
Expand All @@ -55,7 +55,7 @@ import (
var (
// ErrNoGameServerSetOwner is returned when a GameServerSet can't be found as an owner
// for a GameServer
ErrNoGameServerSetOwner = errors.New("No GameServerSet owner for this GameServer")
ErrNoGameServerSetOwner = errs.New("No GameServerSet owner for this GameServer")
)

const (
Expand All @@ -68,6 +68,7 @@ const (
type Extensions struct {
baseLogger *logrus.Entry
apiHooks agonesv1.APIHooks
errs *errors.Errors
}

func init() {
Expand All @@ -89,6 +90,7 @@ type Controller struct {
recorder record.EventRecorder
stateCache *gameServerStateCache
allocationController *AllocationOverflowController
errs *errors.Errors
maxCreationParallelism int
maxGameServerCreationsPerBatch int
maxDeletionParallelism int
Expand Down Expand Up @@ -133,6 +135,7 @@ func NewController(
}

c.baseLogger = runtime.NewLoggerWithType(c)
c.errs = errors.FromStruct(c)
c.workerqueue = workerqueue.NewWorkerQueueWithRateLimiter(c.syncGameServerSet, c.baseLogger, logfields.GameServerSetKey, agones.GroupName+".GameServerSetController", workerqueue.FastRateLimiter(3*time.Second))
health.AddLivenessCheck("gameserverset-workerqueue", healthcheck.Check(c.workerqueue.Healthy))

Expand Down Expand Up @@ -178,6 +181,7 @@ func NewExtensions(apiHooks agonesv1.APIHooks, wh *webhooks.WebHook) *Extensions
ext := &Extensions{apiHooks: apiHooks}

ext.baseLogger = runtime.NewLoggerWithType(ext)
ext.errs = errors.FromStruct(ext)

wh.AddHandler("/validate", agonesv1.Kind("GameServerSet"), admissionv1.Create, ext.creationValidationHandler)
wh.AddHandler("/validate", agonesv1.Kind("GameServerSet"), admissionv1.Update, ext.updateValidationHandler)
Expand All @@ -195,7 +199,7 @@ func (c *Controller) Run(ctx context.Context, workers int) error {

c.baseLogger.Debug("Wait for cache sync")
if !cache.WaitForCacheSync(ctx.Done(), c.gameServerSynced, c.gameServerSetSynced) {
return errors.New("failed to wait for caches to sync")
return c.errs.New("failed to wait for caches to sync")
}

go func() {
Expand All @@ -218,12 +222,12 @@ func (ext *Extensions) updateValidationHandler(review admissionv1.AdmissionRevie

newObj := review.Request.Object
if err := json.Unmarshal(newObj.Raw, newGss); err != nil {
return review, errors.Wrapf(err, "error unmarshalling new GameServerSet json: %s", newObj.Raw)
return review, ext.errs.Wrapf(err, "error unmarshalling new GameServerSet json: %s", newObj.Raw)
}

oldObj := review.Request.OldObject
if err := json.Unmarshal(oldObj.Raw, oldGss); err != nil {
return review, errors.Wrapf(err, "error unmarshalling old GameServerSet json: %s", oldObj.Raw)
return review, ext.errs.Wrapf(err, "error unmarshalling old GameServerSet json: %s", oldObj.Raw)
}

if errs := oldGss.ValidateUpdate(newGss); len(errs) > 0 {
Expand All @@ -249,7 +253,7 @@ func (ext *Extensions) creationValidationHandler(review admissionv1.AdmissionRev

newObj := review.Request.Object
if err := json.Unmarshal(newObj.Raw, newGss); err != nil {
return review, errors.Wrapf(err, "error unmarshalling GameServerSet json after schema validation: %s", newObj.Raw)
return review, ext.errs.Wrapf(err, "error unmarshalling GameServerSet json after schema validation: %s", newObj.Raw)
}

if errs := newGss.Validate(ext.apiHooks); len(errs) > 0 {
Expand Down Expand Up @@ -282,7 +286,7 @@ func (c *Controller) gameServerEventHandler(obj interface{}) {
c.baseLogger.WithField("ref", ref).Debug("Owner GameServerSet no longer available for syncing")
} else {
runtime.HandleError(c.baseLogger.WithField("gsKey", gs.ObjectMeta.Namespace+"/"+gs.ObjectMeta.Name).WithField("ref", ref),
errors.Wrap(err, "error retrieving GameServer owner"))
c.errs.Wrap(err, "error retrieving GameServer owner"))
}
return
}
Expand All @@ -296,7 +300,7 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error {
namespace, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
// don't return an error, as we don't want this retried
runtime.HandleError(loggerForGameServerSetKey(c.baseLogger, key), errors.Wrapf(err, "invalid resource key"))
runtime.HandleError(loggerForGameServerSetKey(c.baseLogger, key), c.errs.Wrapf(err, "invalid resource key"))
return nil
}

Expand All @@ -306,7 +310,7 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error {
loggerForGameServerSetKey(c.baseLogger, key).Debug("GameServerSet is no longer available for syncing")
return nil
}
return errors.Wrapf(err, "error retrieving GameServerSet %s from namespace %s", name, namespace)
return c.errs.Wrapf(err, "error retrieving GameServerSet %s from namespace %s", name, namespace)
}

list, err := ListGameServersByGameServerSetOwner(c.gameServerLister, gsSet)
Expand Down Expand Up @@ -357,14 +361,14 @@ func (c *Controller) syncGameServerSet(ctx context.Context, key string) error {
if numServersToAdd > 0 {
if err := c.addMoreGameServers(ctx, gsSet, numServersToAdd); err != nil {
loggerForGameServerSet(c.baseLogger, gsSet).WithError(err).Warning("error adding game servers")
return errors.Wrap(err, "error adding game servers")
return c.errs.Wrap(err, "error adding game servers")
}
}

if len(toDelete) > 0 {
if err := c.deleteGameServers(ctx, gsSet, toDelete); err != nil {
loggerForGameServerSet(c.baseLogger, gsSet).WithError(err).Warning("error deleting game servers")
return errors.Wrap(err, "error deleting game servers")
return c.errs.Wrap(err, "error deleting game servers")
}
}

Expand Down Expand Up @@ -534,7 +538,7 @@ func (c *Controller) addMoreGameServers(ctx context.Context, gsSet *agonesv1.Gam
return parallelize(newGameServersChannel(count, gsSet), c.maxCreationParallelism, func(gs *agonesv1.GameServer) error {
gs, err := c.gameServerGetter.GameServers(gs.Namespace).Create(ctx, gs, metav1.CreateOptions{})
if err != nil {
return errors.Wrapf(err, "error creating gameserver for gameserverset %s", gsSet.ObjectMeta.Name)
return c.errs.Wrapf(err, "error creating gameserver for gameserverset %s", gsSet.ObjectMeta.Name)
}

c.stateCache.forGameServerSet(gsSet).created(gs)
Expand All @@ -552,7 +556,7 @@ func (c *Controller) deleteGameServers(ctx context.Context, gsSet *agonesv1.Game
gsCopy.Status.State = agonesv1.GameServerStateShutdown
_, err := c.gameServerGetter.GameServers(gs.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{})
if err != nil {
return errors.Wrapf(err, "error updating gameserver %s from status %s to Shutdown status", gs.ObjectMeta.Name, gs.Status.State)
return c.errs.Wrapf(err, "error updating gameserver %s from status %s to Shutdown status", gs.ObjectMeta.Name, gs.Status.State)
}

c.stateCache.forGameServerSet(gsSet).deleted(gs)
Expand Down Expand Up @@ -632,7 +636,7 @@ func (c *Controller) updateStatusIfChanged(ctx context.Context, gsSet *agonesv1.
gsSetCopy.Status = status
_, err := c.gameServerSetGetter.GameServerSets(gsSet.ObjectMeta.Namespace).UpdateStatus(ctx, gsSetCopy, metav1.UpdateOptions{})
if err != nil {
return errors.Wrapf(err, "error updating status on GameServerSet %s", gsSet.ObjectMeta.Name)
return c.errs.Wrapf(err, "error updating status on GameServerSet %s", gsSet.ObjectMeta.Name)
}
}
return nil
Expand Down
15 changes: 8 additions & 7 deletions pkg/gameserversets/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package gameserversets
import (
"context"
"encoding/json"
"errors"
"fmt"
"math/rand"
"net/http"
Expand All @@ -31,8 +32,8 @@ import (
agtesting "agones.dev/agones/pkg/testing"
utilruntime "agones.dev/agones/pkg/util/runtime"
"agones.dev/agones/pkg/util/webhooks"

"github.com/heptiolabs/healthcheck"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -596,7 +597,7 @@ func TestGameServerSetDropCountsAndListsStatus(t *testing.T) {
assert.Nil(t, gsSet.Status.Counters)
assert.Nil(t, gsSet.Status.Lists)
default:
return false, nil, errors.Errorf("Flag string(utilruntime.FeatureCountsAndLists) should be set")
return false, nil, errors.New("Flag string(utilruntime.FeatureCountsAndLists) should be set")
}

return true, gsSet, nil
Expand Down Expand Up @@ -1008,7 +1009,7 @@ func TestControllerSyncUnhealthyGameServers(t *testing.T) {

err := c.deleteGameServers(ctx, gsSet, []*agonesv1.GameServer{gs1, gs2, gs3})
require.Error(t, err)
assert.Contains(t, err.Error(), "error updating gameserver")
assert.ErrorContains(t, err, "error updating gameserver")
})
}

Expand Down Expand Up @@ -1059,7 +1060,7 @@ func TestSyncMoreGameServers(t *testing.T) {

err := c.addMoreGameServers(ctx, gsSet, expected)
require.Error(t, err)
assert.Equal(t, "error creating gameserver for gameserverset test: create-err", err.Error())
assert.ErrorContains(t, err, "error creating gameserver for gameserverset test: create-err")
})
}

Expand Down Expand Up @@ -1180,7 +1181,7 @@ func TestControllerUpdateValidationHandler(t *testing.T) {

_, err := ext.updateValidationHandler(review)
require.Error(t, err)
assert.Equal(t, "error unmarshalling new GameServerSet json: : unexpected end of JSON input", err.Error())
assert.ErrorContains(t, err, "error unmarshalling new GameServerSet json: : unexpected end of JSON input")
})

t.Run("old object is nil, err excpected", func(t *testing.T) {
Expand All @@ -1205,7 +1206,7 @@ func TestControllerUpdateValidationHandler(t *testing.T) {

_, err = ext.updateValidationHandler(review)
require.Error(t, err)
assert.Equal(t, "error unmarshalling old GameServerSet json: : unexpected end of JSON input", err.Error())
assert.ErrorContains(t, err, "error unmarshalling old GameServerSet json: : unexpected end of JSON input")
})

t.Run("invalid gameserverset update", func(t *testing.T) {
Expand Down Expand Up @@ -1311,7 +1312,7 @@ func TestCreationValidationHandler(t *testing.T) {

_, err := ext.creationValidationHandler(review)
require.Error(t, err)
assert.Equal(t, "error unmarshalling GameServerSet json after schema validation: : unexpected end of JSON input", err.Error())
assert.ErrorContains(t, err, "error unmarshalling GameServerSet json after schema validation: : unexpected end of JSON input")
})

t.Run("invalid gameserverset create", func(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions pkg/gameserversets/gameserversets.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ import (
agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
listerv1 "agones.dev/agones/pkg/client/listers/agones/v1"
"agones.dev/agones/pkg/gameservers"
"agones.dev/agones/pkg/util/errors"
"agones.dev/agones/pkg/util/logfields"
"agones.dev/agones/pkg/util/runtime"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
)

var errs = errors.FromPackage()

func loggerForGameServerSetKey(log *logrus.Entry, key string) *logrus.Entry {
return logfields.AugmentLogEntry(log, logfields.GameServerSetKey, key)
}
Expand Down Expand Up @@ -128,7 +130,7 @@ func ListGameServersByGameServerSetOwner(gameServerLister listerv1.GameServerLis
gsSet *agonesv1.GameServerSet) ([]*agonesv1.GameServer, error) {
list, err := gameServerLister.List(labels.SelectorFromSet(labels.Set{agonesv1.GameServerSetGameServerLabel: gsSet.ObjectMeta.Name}))
if err != nil {
return list, errors.Wrapf(err, "error listing gameservers for gameserverset %s", gsSet.ObjectMeta.Name)
return list, errs.Wrapf(err, "error listing gameservers for gameserverset %s", gsSet.ObjectMeta.Name)
}

var result []*agonesv1.GameServer
Expand Down