Skip to content

feat: use new errors pkg in pkg/fleets/* #4710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
markmandel merged 2 commits into from
Sep 5, 2026
+47 −41
Merged

feat: use new errors pkg in pkg/fleets/* #4710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea4392d
feat: use new errors pkg in pkg/fleets/*
lacroixthomas Sep 3, 2026
97a27fd
Merge branch 'main' into features/integrate-errors-fleets-pkg
markmandel Sep 5, 2026
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
40 changes: 22 additions & 18 deletions pkg/fleets/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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!")
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand Down
7 changes: 3 additions & 4 deletions pkg/fleets/controller_rollingupdatefix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
33 changes: 17 additions & 16 deletions pkg/fleets/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package fleets
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"testing"
"time"
Expand All @@ -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"
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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")
})
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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")
})

}
Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -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")
})
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
Loading