diff --git a/pkg/gameserversets/allocation_overflow.go b/pkg/gameserversets/allocation_overflow.go index 01b0594e87..6064d013a6 100644 --- a/pkg/gameserversets/allocation_overflow.go +++ b/pkg/gameserversets/allocation_overflow.go @@ -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" @@ -48,6 +48,7 @@ type AllocationOverflowController struct { gameServerSetSynced cache.InformerSynced gameServerSetLister listerv1.GameServerSetLister workerqueue *workerqueue.WorkerQueue + errs *errors.Errors } // NewAllocatorOverflowController returns a new AllocationOverflowController @@ -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) @@ -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) @@ -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 } @@ -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 @@ -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) } } diff --git a/pkg/gameserversets/controller.go b/pkg/gameserversets/controller.go index d5d7cd48b0..07e278f8d9 100644 --- a/pkg/gameserversets/controller.go +++ b/pkg/gameserversets/controller.go @@ -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" @@ -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 ( @@ -68,6 +68,7 @@ const ( type Extensions struct { baseLogger *logrus.Entry apiHooks agonesv1.APIHooks + errs *errors.Errors } func init() { @@ -89,6 +90,7 @@ type Controller struct { recorder record.EventRecorder stateCache *gameServerStateCache allocationController *AllocationOverflowController + errs *errors.Errors maxCreationParallelism int maxGameServerCreationsPerBatch int maxDeletionParallelism int @@ -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)) @@ -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) @@ -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() { @@ -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 { @@ -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 { @@ -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 } @@ -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 } @@ -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) @@ -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") } } @@ -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) @@ -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) @@ -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 diff --git a/pkg/gameserversets/controller_test.go b/pkg/gameserversets/controller_test.go index 2dd44e7191..5665713a57 100644 --- a/pkg/gameserversets/controller_test.go +++ b/pkg/gameserversets/controller_test.go @@ -17,6 +17,7 @@ package gameserversets import ( "context" "encoding/json" + "errors" "fmt" "math/rand" "net/http" @@ -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" @@ -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 @@ -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") }) } @@ -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") }) } @@ -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) { @@ -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) { @@ -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) { diff --git a/pkg/gameserversets/gameserversets.go b/pkg/gameserversets/gameserversets.go index 2849c84a71..712a1d2edb 100644 --- a/pkg/gameserversets/gameserversets.go +++ b/pkg/gameserversets/gameserversets.go @@ -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) } @@ -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