diff --git a/pkg/fleets/controller.go b/pkg/fleets/controller.go index 780681a431..850eae33b3 100644 --- a/pkg/fleets/controller.go +++ b/pkg/fleets/controller.go @@ -29,13 +29,13 @@ import ( "agones.dev/agones/pkg/client/informers/externalversions" listerv1 "agones.dev/agones/pkg/client/listers/agones/v1" "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" "gomodules.xyz/jsonpatch/v2" admissionv1 "k8s.io/api/admission/v1" @@ -59,11 +59,13 @@ import ( type Extensions struct { baseLogger *logrus.Entry apiHooks agonesv1.APIHooks + errs *errors.Errors } // Controller is a the GameServerSet controller type Controller struct { baseLogger *logrus.Entry + errs *errors.Errors crdGetter apiextclientv1.CustomResourceDefinitionInterface gameServerSynced cache.InformerSynced gameServerSetGetter getterv1.GameServerSetsGetter @@ -107,6 +109,7 @@ func NewController( } c.baseLogger = runtime.NewLoggerWithType(c) + c.errs = errors.FromStruct(c) c.workerqueue = workerqueue.NewWorkerQueueWithRateLimiter(c.syncFleet, c.baseLogger, logfields.FleetKey, agones.GroupName+".FleetController", workerqueue.FastRateLimiter(3*time.Second)) health.AddLivenessCheck("fleet-workerqueue", healthcheck.Check(c.workerqueue.Healthy)) @@ -166,6 +169,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("/mutate", agonesv1.Kind("Fleet"), admissionv1.Create, ext.creationMutationHandler) wh.AddHandler("/validate", agonesv1.Kind("Fleet"), admissionv1.Create, ext.creationValidationHandler) @@ -196,17 +200,17 @@ func (ext *Extensions) creationMutationHandler(review admissionv1.AdmissionRevie newFleet, err := json.Marshal(fleet) if err != nil { - return review, errors.Wrapf(err, "error marshalling default applied Fleet %s to json", fleet.ObjectMeta.Name) + return review, ext.errs.Wrapf(err, "error marshalling default applied Fleet %s to json", fleet.ObjectMeta.Name) } patch, err := jsonpatch.CreatePatch(obj.Raw, newFleet) if err != nil { - return review, errors.Wrapf(err, "error creating patch for Fleet %s", fleet.ObjectMeta.Name) + return review, ext.errs.Wrapf(err, "error creating patch for Fleet %s", fleet.ObjectMeta.Name) } jsn, err := json.Marshal(patch) if err != nil { - return review, errors.Wrapf(err, "error creating json for patch for Fleet %s", fleet.ObjectMeta.Name) + return review, ext.errs.Wrapf(err, "error creating json for patch for Fleet %s", fleet.ObjectMeta.Name) } loggerForFleet(fleet, ext.baseLogger).WithField("patch", string(jsn)).Debug("patch created!") @@ -227,7 +231,7 @@ func (ext *Extensions) creationValidationHandler(review admissionv1.AdmissionRev fleet := &agonesv1.Fleet{} err := json.Unmarshal(obj.Raw, fleet) if err != nil { - return review, errors.Wrapf(err, "error unmarshalling Fleet json after schema validation: %s", obj.Raw) + return review, ext.errs.Wrapf(err, "error unmarshalling Fleet json after schema validation: %s", obj.Raw) } if errs := fleet.Validate(ext.apiHooks); len(errs) > 0 { @@ -254,7 +258,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, c.fleetSynced) { - 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, workers) @@ -290,7 +294,7 @@ func (c *Controller) gameServerSetEventHandler(obj interface{}) { c.baseLogger.WithField("ref", ref).Warn("Owner Fleet no longer available for syncing") } else { runtime.HandleError(loggerForFleet(fleet, c.baseLogger).WithField("ref", ref), - errors.Wrap(err, "error retrieving GameServerSet owner")) + c.errs.Wrap(err, "error retrieving GameServerSet owner")) } return } @@ -306,7 +310,7 @@ func (c *Controller) syncFleet(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(loggerForFleetKey(key, c.baseLogger), errors.Wrapf(err, "invalid resource key")) + runtime.HandleError(loggerForFleetKey(key, c.baseLogger), c.errs.Wrapf(err, "invalid resource key")) return nil } @@ -316,7 +320,7 @@ func (c *Controller) syncFleet(ctx context.Context, key string) error { loggerForFleetKey(key, c.baseLogger).Debug("Fleet is no longer available for syncing") return nil } - return errors.Wrapf(err, "error retrieving fleet %s from namespace %s", name, namespace) + return c.errs.Wrapf(err, "error retrieving fleet %s from namespace %s", name, namespace) } // If Fleet is marked for deletion don't do anything. @@ -361,7 +365,7 @@ func (c *Controller) upsertGameServerSet(ctx context.Context, fleet *agonesv1.Fl gsSets := c.gameServerSetGetter.GameServerSets(active.ObjectMeta.Namespace) gsSet, err := gsSets.Create(ctx, active, metav1.CreateOptions{}) if err != nil { - return errors.Wrapf(err, "error creating gameserverset for fleet %s", fleet.ObjectMeta.Name) + return c.errs.Wrapf(err, "error creating gameserverset for fleet %s", fleet.ObjectMeta.Name) } // extra step which is needed to set @@ -372,7 +376,7 @@ func (c *Controller) upsertGameServerSet(ctx context.Context, fleet *agonesv1.Fl gsSetCopy.Status.AllocatedReplicas = 0 _, err = gsSets.UpdateStatus(ctx, gsSetCopy, metav1.UpdateOptions{}) if err != nil { - return errors.Wrapf(err, "error updating status of gameserverset for fleet %s", + return c.errs.Wrapf(err, "error updating status of gameserverset for fleet %s", fleet.ObjectMeta.Name) } @@ -387,7 +391,7 @@ func (c *Controller) upsertGameServerSet(ctx context.Context, fleet *agonesv1.Fl gsSetCopy.Spec.Scheduling = fleet.Spec.Scheduling gsSetCopy, err := c.gameServerSetGetter.GameServerSets(fleet.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}) if err != nil { - return errors.Wrapf(err, "error updating replicas for gameserverset for fleet %s", fleet.ObjectMeta.Name) + return c.errs.Wrapf(err, "error updating replicas for gameserverset for fleet %s", fleet.ObjectMeta.Name) } c.recorder.Eventf(fleet, corev1.EventTypeNormal, "ScalingGameServerSet", "Scaling active GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, active.Spec.Replicas, gsSetCopy.Spec.Replicas) @@ -400,7 +404,7 @@ func (c *Controller) upsertGameServerSet(ctx context.Context, fleet *agonesv1.Fl gsSetCopy.Spec.Priorities = fleet.Spec.Priorities _, err := c.gameServerSetGetter.GameServerSets(fleet.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}) if err != nil { - return errors.Wrapf(err, "error updating priorities for gameserverset for fleet %s", fleet.ObjectMeta.Name) + return c.errs.Wrapf(err, "error updating priorities for gameserverset for fleet %s", fleet.ObjectMeta.Name) } c.recorder.Eventf(fleet, corev1.EventTypeNormal, "UpdatingGameServerSet", "Updated GameServerSet %s Priorities", gsSetCopy.ObjectMeta.Name) @@ -440,7 +444,7 @@ func (c *Controller) applyDeploymentStrategy(ctx context.Context, fleet *agonesv return c.rollingUpdateDeployment(ctx, fleet, active, rest) } - return 0, errors.Errorf("unexpected deployment strategy type: %s", fleet.Spec.Strategy.Type) + return 0, c.errs.Errorf("unexpected deployment strategy type: %s", fleet.Spec.Strategy.Type) } // deleteEmptyGameServerSets deletes all GameServerServerSets @@ -451,7 +455,7 @@ func (c *Controller) deleteEmptyGameServerSets(ctx context.Context, fleet *agone if gsSet.Status.Replicas == 0 && gsSet.Status.ShutdownReplicas == 0 { err := c.gameServerSetGetter.GameServerSets(gsSet.ObjectMeta.Namespace).Delete(ctx, gsSet.ObjectMeta.Name, metav1.DeleteOptions{PropagationPolicy: &p}) if err != nil { - return errors.Wrapf(err, "error updating gameserverset %s", gsSet.ObjectMeta.Name) + return c.errs.Wrapf(err, "error updating gameserverset %s", gsSet.ObjectMeta.Name) } c.recorder.Eventf(fleet, corev1.EventTypeNormal, "DeletingGameServerSet", "Deleting inactive GameServerSet %s", gsSet.ObjectMeta.Name) @@ -472,7 +476,7 @@ func (c *Controller) recreateDeployment(ctx context.Context, fleet *agonesv1.Fle gsSetCopy := gsSet.DeepCopy() gsSetCopy.Spec.Replicas = 0 if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}); err != nil { - return 0, errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) + return 0, c.errs.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) @@ -528,7 +532,7 @@ func (c *Controller) rollingUpdateActive(fleet *agonesv1.Fleet, active *agonesv1 r, err := intstr.GetValueFromIntOrPercent(fleet.Spec.Strategy.RollingUpdate.MaxSurge, int(fleet.Spec.Replicas), true) if err != nil { - return 0, errors.Wrapf(err, "error parsing MaxSurge value: %s", fleet.ObjectMeta.Name) + return 0, c.errs.Wrapf(err, "error parsing MaxSurge value: %s", fleet.ObjectMeta.Name) } surge := int32(r) @@ -624,7 +628,7 @@ func (c *Controller) updateFleetStatus(ctx context.Context, fleet *agonesv1.Flee _, err = c.fleetGetter.Fleets(fCopy.ObjectMeta.Namespace).UpdateStatus(ctx, fCopy, metav1.UpdateOptions{}) if err != nil { - return errors.Wrapf(err, "error updating status of fleet %s", fCopy.ObjectMeta.Name) + return c.errs.Wrapf(err, "error updating status of fleet %s", fCopy.ObjectMeta.Name) } // The update was successful, the allocation count must be decremented to reflect this. diff --git a/pkg/fleets/controller_rollingupdatefix.go b/pkg/fleets/controller_rollingupdatefix.go index b1b9d18711..833acdcaec 100644 --- a/pkg/fleets/controller_rollingupdatefix.go +++ b/pkg/fleets/controller_rollingupdatefix.go @@ -5,7 +5,6 @@ import ( "fmt" agonesv1 "agones.dev/agones/pkg/apis/agones/v1" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -42,7 +41,7 @@ func (c *Controller) cleanupUnhealthyReplicasRollingUpdateFix(ctx context.Contex gsSetCopy.Spec.Replicas = newReplicasCount totalScaledDown += scaledDownCount if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}); err != nil { - return nil, totalScaledDown, errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) + return nil, totalScaledDown, c.errs.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) @@ -60,7 +59,7 @@ func (c *Controller) rollingUpdateRestFixedOnReadyRollingUpdateFix(ctx context.C // Look at Kubernetes Deployment util ResolveFenceposts() function r, err := intstr.GetValueFromIntOrPercent(fleet.Spec.Strategy.RollingUpdate.MaxUnavailable, int(fleet.Status.ReadyReplicas), false) if err != nil { - return errors.Wrapf(err, "error parsing MaxUnavailable value: %s", fleet.ObjectMeta.Name) + return c.errs.Wrapf(err, "error parsing MaxUnavailable value: %s", fleet.ObjectMeta.Name) } if r == 0 { r = 1 @@ -150,7 +149,7 @@ func (c *Controller) rollingUpdateRestFixedOnReadyRollingUpdateFix(ctx context.C Debug("applying rolling update to inactive gameserverset") if _, err := c.gameServerSetGetter.GameServerSets(gsSetCopy.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{}); err != nil { - return errors.Wrapf(err, "error updating gameserverset %s", gsSetCopy.ObjectMeta.Name) + return c.errs.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) diff --git a/pkg/fleets/controller_test.go b/pkg/fleets/controller_test.go index b02bdf7c1b..5ea0088047 100644 --- a/pkg/fleets/controller_test.go +++ b/pkg/fleets/controller_test.go @@ -18,6 +18,8 @@ package fleets import ( "context" "encoding/json" + "errors" + "fmt" "net/http" "testing" "time" @@ -32,7 +34,6 @@ import ( 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" @@ -239,7 +240,7 @@ func TestControllerSyncFleet(t *testing.T) { c.fleetLister = &fakeFleetListerWithErr{} err := c.syncFleet(context.Background(), "default/fleet-1") - assert.EqualError(t, err, "error retrieving fleet fleet-1 from namespace default: err-from-namespace-lister") + assert.ErrorContains(t, err, "error retrieving fleet fleet-1 from namespace default: err-from-namespace-lister") }) t.Run("error on getting list of GS", func(t *testing.T) { @@ -255,7 +256,7 @@ func TestControllerSyncFleet(t *testing.T) { defer cancel() err := c.syncFleet(ctx, "default/fleet-1") - assert.EqualError(t, err, "error listing gameserversets for fleet fleet-1: random-err") + assert.ErrorContains(t, err, "error listing gameserversets for fleet fleet-1: random-err") }) t.Run("fleet not found", func(t *testing.T) { @@ -288,7 +289,7 @@ func TestControllerSyncFleet(t *testing.T) { defer cancel() err := c.syncFleet(ctx, "default/fleet-1") - assert.EqualError(t, err, "unexpected deployment strategy type: invalid-strategy-type") + assert.ErrorContains(t, err, "unexpected deployment strategy type: invalid-strategy-type") }) t.Run("error on deleteEmptyGameServerSets", func(t *testing.T) { @@ -315,7 +316,7 @@ func TestControllerSyncFleet(t *testing.T) { defer cancel() err := c.syncFleet(ctx, "default/fleet-1") - assert.EqualError(t, err, "error updating gameserverset : random-err") + assert.ErrorContains(t, err, "error updating gameserverset : random-err") }) t.Run("error on upsertGameServerSet", func(t *testing.T) { @@ -340,7 +341,7 @@ func TestControllerSyncFleet(t *testing.T) { defer cancel() err := c.syncFleet(ctx, "default/fleet-1") - assert.EqualError(t, err, "error creating gameserverset for fleet fleet-1: random-err") + assert.ErrorContains(t, err, "error creating gameserverset for fleet fleet-1: random-err") }) } @@ -355,7 +356,7 @@ func TestControllerCreationValidationHandler(t *testing.T) { review := getAdmissionReview(raw) _, err = ext.creationValidationHandler(review) - assert.EqualError(t, err, "error unmarshalling Fleet json after schema validation: \"MQ==\": json: cannot unmarshal string into Go value of type v1.Fleet") + assert.ErrorContains(t, err, "error unmarshalling Fleet json after schema validation: \"MQ==\": json: cannot unmarshal string into Go value of type v1.Fleet") }) t.Run("invalid fleet", func(t *testing.T) { @@ -553,7 +554,7 @@ func TestControllerUpdateFleetStatus(t *testing.T) { c.gameServerSetLister = &fakeGSSListerWithErr{} err := c.updateFleetStatus(context.Background(), fleet) - assert.EqualError(t, err, "error listing gameserversets for fleet fleet-1: random-err") + assert.ErrorContains(t, err, "error listing gameserversets for fleet fleet-1: random-err") }) t.Run("fleets getter returns an error", func(t *testing.T) { @@ -564,7 +565,7 @@ func TestControllerUpdateFleetStatus(t *testing.T) { err := c.updateFleetStatus(context.Background(), fleet) - assert.EqualError(t, err, "err-from-fleet-getter") + assert.ErrorContains(t, err, "err-from-fleet-getter") }) } @@ -860,7 +861,7 @@ func TestFleetDropCountsAndListsStatus(t *testing.T) { assert.Nil(t, fleet.Status.Counters) assert.Nil(t, fleet.Status.Lists) default: - return false, fleet, errors.Errorf("Flag string(utilruntime.FeatureCountsAndLists) should be set") + return false, fleet, errors.New("Flag string(utilruntime.FeatureCountsAndLists) should be set") } return true, fleet, nil }) @@ -1013,7 +1014,7 @@ func TestControllerRecreateDeployment(t *testing.T) { _, err := c.recreateDeployment(context.Background(), f, []*agonesv1.GameServerSet{gsSet1, gsSet2}) - assert.EqualError(t, err, "error updating gameserverset gsSet1: random-err") + assert.ErrorContains(t, err, "error updating gameserverset gsSet1: random-err") }) } @@ -1111,7 +1112,7 @@ func TestControllerUpsertGameServerSet(t *testing.T) { err := c.upsertGameServerSet(context.Background(), f, gsSet, replicas) - assert.EqualError(t, err, "error updating replicas for gameserverset for fleet fleet-1: random-err") + assert.ErrorContains(t, err, "error updating replicas for gameserverset for fleet fleet-1: random-err") }) t.Run("error on gs status update", func(t *testing.T) { @@ -1124,7 +1125,7 @@ func TestControllerUpsertGameServerSet(t *testing.T) { err := c.upsertGameServerSet(context.Background(), f, gsSet, replicas) - assert.EqualError(t, err, "error updating status of gameserverset for fleet fleet-1: random-err") + assert.ErrorContains(t, err, "error updating status of gameserverset for fleet fleet-1: random-err") }) t.Run("nothing happens, nil is returned", func(t *testing.T) { @@ -1303,7 +1304,7 @@ func TestControllerRollingUpdateDeploymentNegativeReplica(t *testing.T) { assert.Equal(t, int32(4), gsSet.Spec.Replicas) assert.Equal(t, int32(5), f.Spec.Replicas) - return true, nil, errors.Errorf("error updating replicas for gameserverset for fleet %s", f.Name) + return true, nil, fmt.Errorf("error updating replicas for gameserverset for fleet %s", f.Name) }) // assert the active gameserverset's replicas when active and inactive gameserversets exist @@ -1403,7 +1404,7 @@ func TestControllerRollingUpdateDeploymentGSSUpdateFailedErrExpected(t *testing. }) _, err := c.rollingUpdateDeployment(context.Background(), f, active, []*agonesv1.GameServerSet{inactive}) - assert.EqualError(t, err, "error updating gameserverset inactive: random-err") + assert.ErrorContains(t, err, "error updating gameserverset inactive: random-err") } func TestRollingUpdateOnReady(t *testing.T) { @@ -1721,7 +1722,7 @@ func TestControllerRollingUpdateDeployment(t *testing.T) { replicas, err := c.rollingUpdateDeployment(context.Background(), f, active, []*agonesv1.GameServerSet{inactive}) if v.expected.err != "" { - assert.EqualError(t, err, v.expected.err) + assert.ErrorContains(t, err, v.expected.err) } else { require.NoError(t, err) assert.Equal(t, v.expected.replicas, replicas) diff --git a/pkg/fleets/fleets.go b/pkg/fleets/fleets.go index 5d1b92094e..812b33c824 100644 --- a/pkg/fleets/fleets.go +++ b/pkg/fleets/fleets.go @@ -19,17 +19,19 @@ package fleets import ( agonesv1 "agones.dev/agones/pkg/apis/agones/v1" listerv1 "agones.dev/agones/pkg/client/listers/agones/v1" - "github.com/pkg/errors" + "agones.dev/agones/pkg/util/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" ) +var errs = errors.FromPackage() + // ListGameServerSetsByFleetOwner lists all the GameServerSets for a given // Fleet func ListGameServerSetsByFleetOwner(gameServerSetNamespacedLister listerv1.GameServerSetNamespaceLister, f *agonesv1.Fleet) ([]*agonesv1.GameServerSet, error) { list, err := gameServerSetNamespacedLister.List(labels.SelectorFromSet(labels.Set{agonesv1.FleetNameLabel: f.ObjectMeta.Name})) if err != nil { - return list, errors.Wrapf(err, "error listing gameserversets for fleet %s", f.ObjectMeta.Name) + return list, errs.Wrapf(err, "error listing gameserversets for fleet %s", f.ObjectMeta.Name) } var result []*agonesv1.GameServerSet @@ -49,7 +51,7 @@ func ListGameServersByFleetOwner(gameServerNamespacedLister listerv1.GameServerN list, err := gameServerNamespacedLister.List(labels.SelectorFromSet(labels.Set{agonesv1.FleetNameLabel: fleet.ObjectMeta.Name})) if err != nil { - return list, errors.Wrapf(err, "error listing gameservers for fleets %s", fleet.ObjectMeta.Name) + return list, errs.Wrapf(err, "error listing gameservers for fleets %s", fleet.ObjectMeta.Name) } return list, nil }