From ae030c938ee3ae7fda7e07c7d6c366074e0a8389 Mon Sep 17 00:00:00 2001 From: igooch Date: Wed, 8 May 2024 16:53:20 -0700 Subject: [PATCH 1/5] Changes `sdk-server` to Patch instead of Update (#3803) * Changes UpdateLabels to Patch instead of Update * SDK update Counters to use Patch instead of Update * SDK update Lists uses Patch instead of Update * SDK update annotations and state uses Patch instead of Update * Changes AssertEqual on a time for AssertWithinDuration of 1 second * Adds copy of game server to TestSDKServerReserveTimeout patch reactor * Pulls out game server patch into helper function * Changes the SDK role binding permission from Update to Patch * SDK server update PlayerTracking to patch * Unexports helper function * Update game server Patch to fail if the resource version is changed or not set * Adds e2e test for game server patch * Increases the SDKServerReserveTimeout tolerance for difference between when the game server is reserved to when the test checks that the game server has been reserved from 1 second to 1.5 seconds * Updates beta variable to use b instead of the alpha a * Caches updated version of game server in the SDK --- .../agones/templates/serviceaccounts/sdk.yaml | 2 +- install/yaml/install.yaml | 2 +- pkg/apis/agones/v1/gameserver.go | 14 +- pkg/apis/agones/v1/gameserver_test.go | 3 +- pkg/sdkserver/sdkserver.go | 47 +- pkg/sdkserver/sdkserver_test.go | 481 ++++++++++-------- sdks/go/beta.go | 52 +- sdks/go/beta_test.go | 116 ++--- test/e2e/gameserver_test.go | 59 +++ 9 files changed, 442 insertions(+), 334 deletions(-) diff --git a/install/helm/agones/templates/serviceaccounts/sdk.yaml b/install/helm/agones/templates/serviceaccounts/sdk.yaml index abe5d27da8..0137c82141 100644 --- a/install/helm/agones/templates/serviceaccounts/sdk.yaml +++ b/install/helm/agones/templates/serviceaccounts/sdk.yaml @@ -45,7 +45,7 @@ rules: verbs: ["create", "patch"] - apiGroups: ["agones.dev"] resources: ["gameservers"] - verbs: ["list", "update", "watch"] + verbs: ["list", "patch", "watch"] --- {{- range .Values.gameservers.namespaces }} apiVersion: rbac.authorization.k8s.io/v1 diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml index 892f29f710..605eaaa5ec 100644 --- a/install/yaml/install.yaml +++ b/install/yaml/install.yaml @@ -16691,7 +16691,7 @@ rules: verbs: ["create", "patch"] - apiGroups: ["agones.dev"] resources: ["gameservers"] - verbs: ["list", "update", "watch"] + verbs: ["list", "patch", "watch"] --- # Source: agones/templates/service/allocation.yaml # Bind the agones-allocator ServiceAccount to the agones-allocator ClusterRole diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index 66849654ad..e3fa81e2db 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -879,8 +879,10 @@ func (gs *GameServer) CountPortsForRange(name string, f func(policy PortPolicy) return count } -// Patch creates a JSONPatch to move the current GameServer -// to the passed in delta GameServer +// Patch creates a JSONPatch to move the current GameServer to the passed in delta GameServer. +// Returned Patch includes a "test" operation that will cause the GameServers.Patch() operation to +// fail if the Game Server has been updated (ResourceVersion has changed) in between when the Patch +// was created and applied. func (gs *GameServer) Patch(delta *GameServer) ([]byte, error) { var result []byte @@ -899,7 +901,13 @@ func (gs *GameServer) Patch(delta *GameServer) ([]byte, error) { return result, errors.Wrapf(err, "error creating patch for GameServer %s", gs.ObjectMeta.Name) } - result, err = json.Marshal(patch) + // Per https://jsonpatch.com/ "Tests that the specified value is set in the document. If the test + // fails, then the patch as a whole should not apply." + // Used here to check the object has not been updated (has not changed ResourceVersion). + patches := []jsonpatch.JsonPatchOperation{{Operation: "test", Path: "/metadata/resourceVersion", Value: gs.ObjectMeta.ResourceVersion}} + patches = append(patches, patch...) + + result, err = json.Marshal(patches) return result, errors.Wrapf(err, "error creating json for patch for GameServer %s", gs.ObjectMeta.Name) } diff --git a/pkg/apis/agones/v1/gameserver_test.go b/pkg/apis/agones/v1/gameserver_test.go index 448907a00b..248e14b27e 100644 --- a/pkg/apis/agones/v1/gameserver_test.go +++ b/pkg/apis/agones/v1/gameserver_test.go @@ -1451,7 +1451,7 @@ func TestGameServerCountPortsForRange(t *testing.T) { } func TestGameServerPatch(t *testing.T) { - fixture := &GameServer{ObjectMeta: metav1.ObjectMeta{Name: "lucy"}, + fixture := &GameServer{ObjectMeta: metav1.ObjectMeta{Name: "lucy", ResourceVersion: "1234"}, Spec: GameServerSpec{Container: "goat"}} delta := fixture.DeepCopy() @@ -1461,6 +1461,7 @@ func TestGameServerPatch(t *testing.T) { assert.Nil(t, err) assert.Contains(t, string(patch), `{"op":"replace","path":"/spec/container","value":"bear"}`) + assert.Contains(t, string(patch), `{"op":"test","path":"/metadata/resourceVersion","value":"1234"}`) } func TestGameServerGetDevAddress(t *testing.T) { diff --git a/pkg/sdkserver/sdkserver.go b/pkg/sdkserver/sdkserver.go index abffd27bee..eee6588455 100644 --- a/pkg/sdkserver/sdkserver.go +++ b/pkg/sdkserver/sdkserver.go @@ -29,6 +29,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -355,7 +356,6 @@ func (s *SDKServer) updateState(ctx context.Context) error { } s.gsUpdateMutex.RUnlock() - gameServers := s.gameServerGetter.GameServers(s.namespace) gs, err := s.gameServer() if err != nil { return err @@ -406,7 +406,7 @@ func (s *SDKServer) updateState(ctx context.Context) error { gsCopy.ObjectMeta.Annotations[gameserverallocations.LastAllocatedAnnotationKey] = string(ts) } - gs, err = gameServers.Update(ctx, gsCopy, metav1.UpdateOptions{}) + gs, err = s.patchGameServer(ctx, gs, gsCopy) if err != nil { return errors.Wrapf(err, "could not update GameServer %s/%s to state %s", s.namespace, s.gameServerName, gsCopy.Status.State) } @@ -449,6 +449,17 @@ func (s *SDKServer) gameServer() (*agonesv1.GameServer, error) { return gs, nil } +// patchGameServer is a helper function to create and apply a patch update, so the changes in +// gsCopy are applied to the original gs. +func (s *SDKServer) patchGameServer(ctx context.Context, gs, gsCopy *agonesv1.GameServer) (*agonesv1.GameServer, error) { + patch, err := gs.Patch(gsCopy) + if err != nil { + return nil, err + } + + return s.gameServerGetter.GameServers(s.namespace).Patch(ctx, gs.GetObjectMeta().GetName(), types.JSONPatchType, patch, metav1.PatchOptions{}) +} + // updateLabels updates the labels on this GameServer to the ones persisted in SDKServer, // i.e. SDKServer.gsLabels, with the prefix of "agones.dev/sdk-" func (s *SDKServer) updateLabels(ctx context.Context) error { @@ -469,7 +480,7 @@ func (s *SDKServer) updateLabels(ctx context.Context) error { } s.gsUpdateMutex.RUnlock() - _, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) + _, err = s.patchGameServer(ctx, gs, gsCopy) return err } @@ -493,7 +504,7 @@ func (s *SDKServer) updateAnnotations(ctx context.Context) error { } s.gsUpdateMutex.RUnlock() - _, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) + _, err = s.patchGameServer(ctx, gs, gsCopy) return err } @@ -815,7 +826,7 @@ func (s *SDKServer) GetCounter(ctx context.Context, in *beta.GetCounterRequest) return nil, errors.Errorf("counter not found: %s", in.Name) } s.logger.WithField("Get Counter", counter).Debugf("Got Counter %s", in.Name) - protoCounter := beta.Counter{Name: in.Name, Count: counter.Count, Capacity: counter.Capacity} + protoCounter := &beta.Counter{Name: in.Name, Count: counter.Count, Capacity: counter.Capacity} // If there are batched changes that have not yet been applied, apply them to the Counter. // This does NOT validate batched the changes. if counterUpdate, ok := s.gsCounterUpdates[in.Name]; ok { @@ -837,7 +848,7 @@ func (s *SDKServer) GetCounter(ctx context.Context, in *beta.GetCounterRequest) s.logger.WithField("Get Counter", counter).Debugf("Applied Batched Counter Updates %v", counterUpdate) } - return &protoCounter, nil + return protoCounter, nil } // UpdateCounter collapses all UpdateCounterRequests for a given Counter into a single request. @@ -973,7 +984,7 @@ func (s *SDKServer) updateCounter(ctx context.Context) error { names = append(names, name) } - gs, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) + gs, err = s.patchGameServer(ctx, gs, gsCopy) if err != nil { return err } @@ -1204,7 +1215,7 @@ func (s *SDKServer) updateList(ctx context.Context) error { names = append(names, name) } - gs, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) + gs, err = s.patchGameServer(ctx, gs, gsCopy) if err != nil { return err } @@ -1357,12 +1368,12 @@ func (s *SDKServer) updatePlayerCapacity(ctx context.Context) error { gsCopy.Status.Players.Capacity = s.gsPlayerCapacity s.gsUpdateMutex.RUnlock() - gs, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) - if err != nil { - return err + gs, err = s.patchGameServer(ctx, gs, gsCopy) + if err == nil { + s.recorder.Event(gs, corev1.EventTypeNormal, "PlayerCapacity", fmt.Sprintf("Set to %d", gs.Status.Players.Capacity)) } - s.recorder.Event(gs, corev1.EventTypeNormal, "PlayerCapacity", fmt.Sprintf("Set to %d", gs.Status.Players.Capacity)) - return nil + + return err } // updateConnectedPlayers updates the Player IDs and Count fields in the GameServer's Status. @@ -1390,12 +1401,12 @@ func (s *SDKServer) updateConnectedPlayers(ctx context.Context) error { return nil } - gs, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) - if err != nil { - return err + gs, err = s.patchGameServer(ctx, gs, gsCopy) + if err == nil { + s.recorder.Event(gs, corev1.EventTypeNormal, "PlayerCount", fmt.Sprintf("Set to %d", gs.Status.Players.Count)) } - s.recorder.Event(gs, corev1.EventTypeNormal, "PlayerCount", fmt.Sprintf("Set to %d", gs.Status.Players.Count)) - return nil + + return err } // NewSDKServerContext returns a Context that cancels when SIGTERM or os.Interrupt diff --git a/pkg/sdkserver/sdkserver_test.go b/pkg/sdkserver/sdkserver_test.go index de57c12a86..8809db8488 100644 --- a/pkg/sdkserver/sdkserver_test.go +++ b/pkg/sdkserver/sdkserver_test.go @@ -16,6 +16,7 @@ package sdkserver import ( "context" + "encoding/json" "net/http" "strconv" "sync" @@ -29,6 +30,7 @@ import ( "agones.dev/agones/pkg/sdk/beta" agtesting "agones.dev/agones/pkg/testing" agruntime "agones.dev/agones/pkg/util/runtime" + jsonpatch "github.com/evanphx/json-patch" "github.com/google/go-cmp/cmp" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -45,6 +47,24 @@ import ( testclocks "k8s.io/utils/clock/testing" ) +// patchGameServer is a helper function for the AddReactor "patch" that creates and applies a patch +// to a gameserver. Returns a patched copy and does not modify the original game server. +func patchGameServer(t *testing.T, action k8stesting.Action, gs *agonesv1.GameServer) *agonesv1.GameServer { + pa := action.(k8stesting.PatchAction) + patchJSON := pa.GetPatch() + patch, err := jsonpatch.DecodePatch(patchJSON) + assert.NoError(t, err) + gsCopy := gs.DeepCopy() + gsJSON, err := json.Marshal(gsCopy) + assert.NoError(t, err) + patchedGs, err := patch.Apply(gsJSON) + assert.NoError(t, err) + err = json.Unmarshal(patchedGs, &gsCopy) + assert.NoError(t, err) + + return gsCopy +} + func TestSidecarRun(t *testing.T) { t.Parallel() @@ -151,38 +171,38 @@ func TestSidecarRun(t *testing.T) { m := agtesting.NewMocks() done := make(chan bool) + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Spec: agonesv1.GameServerSpec{ + Health: agonesv1.Health{Disabled: false, FailureThreshold: 1, PeriodSeconds: 1, InitialDelaySeconds: 0}, + }, + Status: agonesv1.GameServerStatus{ + State: agonesv1.GameServerStateStarting, + }, + } + gs.ApplyDefaults() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", - }, - Spec: agonesv1.GameServerSpec{ - Health: agonesv1.Health{Disabled: false, FailureThreshold: 1, PeriodSeconds: 1, InitialDelaySeconds: 0}, - }, - Status: agonesv1.GameServerStatus{ - State: agonesv1.GameServerStateStarting, - }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { defer close(done) - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) + + gsCopy := patchGameServer(t, action, &gs) if v.expected.state != "" { - assert.Equal(t, v.expected.state, gs.Status.State) + assert.Equal(t, v.expected.state, gsCopy.Status.State) } - for label, value := range v.expected.labels { - assert.Equal(t, value, gs.ObjectMeta.Labels[label]) + assert.Equal(t, value, gsCopy.ObjectMeta.Labels[label]) } for ann, value := range v.expected.annotations { - assert.Equal(t, value, gs.ObjectMeta.Annotations[ann]) + assert.Equal(t, value, gsCopy.ObjectMeta.Annotations[ann]) } - - return true, gs, nil + return true, gsCopy, nil }) sc, err := NewSDKServer("test", "default", m.KubeClient, m.AgonesClient, logrus.DebugLevel) @@ -288,31 +308,30 @@ func TestSDKServerSyncGameServer(t *testing.T) { sc.gsAnnotations = v.scData.gsAnnotations updated := false + gs := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{ + UID: "1234", + Name: sc.gameServerName, Namespace: sc.namespace, ResourceVersion: "0", + Labels: map[string]string{}, Annotations: map[string]string{}}, + } m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{ - UID: "1234", - Name: sc.gameServerName, Namespace: sc.namespace, - Labels: map[string]string{}, Annotations: map[string]string{}}, - } - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - updated = true - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) if v.expected.state != "" { - assert.Equal(t, v.expected.state, gs.Status.State) + assert.Equal(t, v.expected.state, gsCopy.Status.State) } for label, value := range v.expected.labels { - assert.Equal(t, value, gs.ObjectMeta.Labels[label]) + assert.Equal(t, value, gsCopy.ObjectMeta.Labels[label]) } for ann, value := range v.expected.annotations { - assert.Equal(t, value, gs.ObjectMeta.Annotations[ann]) + assert.Equal(t, value, gsCopy.ObjectMeta.Annotations[ann]) } - - return true, gs, nil + updated = true + return false, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -363,7 +382,7 @@ func TestSidecarUpdateState(t *testing.T) { m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{Name: sc.gameServerName, Namespace: sc.namespace}, + ObjectMeta: metav1.ObjectMeta{Name: sc.gameServerName, Namespace: sc.namespace, ResourceVersion: "0"}, Status: agonesv1.GameServerStatus{}, } @@ -372,7 +391,7 @@ func TestSidecarUpdateState(t *testing.T) { return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { updated = true return true, nil, nil }) @@ -446,23 +465,25 @@ func TestSidecarUnhealthyMessage(t *testing.T) { sc, err := NewSDKServer("test", "default", m.KubeClient, m.AgonesClient, logrus.DebugLevel) require.NoError(t, err) + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Spec: agonesv1.GameServerSpec{}, + Status: agonesv1.GameServerStatus{ + State: agonesv1.GameServerStateStarting, + }, + } + gs.ApplyDefaults() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", - }, - Spec: agonesv1.GameServerSpec{}, - Status: agonesv1.GameServerStatus{ - State: agonesv1.GameServerStateStarting, - }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - return true, gs, nil + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + + return true, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -916,28 +937,30 @@ func TestSDKServerReserveTimeoutOnRun(t *testing.T) { updated := make(chan agonesv1.GameServerStatus, 1) + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Status: agonesv1.GameServerStatus{ + State: agonesv1.GameServerStateReserved, + }, + } + gs.ApplyDefaults() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { n := metav1.NewTime(metav1.Now().Add(time.Second)) + gsCopy := gs.DeepCopy() + gsCopy.Status.ReservedUntil = &n - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", - }, - Status: agonesv1.GameServerStatus{ - State: agonesv1.GameServerStateReserved, - ReservedUntil: &n, - }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gsCopy}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - updated <- gs.Status + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) - return true, gs, nil + updated <- gsCopy.Status + + return true, gsCopy, nil }) sc, err := defaultSidecar(m) @@ -976,24 +999,23 @@ func TestSDKServerReserveTimeout(t *testing.T) { state := make(chan agonesv1.GameServerStatus, 100) defer close(state) + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Spec: agonesv1.GameServerSpec{Health: agonesv1.Health{Disabled: true}}, + } + gs.ApplyDefaults() + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", - }, - Spec: agonesv1.GameServerSpec{Health: agonesv1.Health{Disabled: true}}, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - - state <- gs.Status + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) - return true, gs, nil + state <- gsCopy.Status + return true, gsCopy, nil }) sc, err := defaultSidecar(m) @@ -1013,18 +1035,18 @@ func TestSDKServerReserveTimeout(t *testing.T) { wg.Done() }() - assertStateChange := func(expected agonesv1.GameServerState, timeout time.Duration, additional func(status agonesv1.GameServerStatus)) { + assertStateChange := func(expected agonesv1.GameServerState, additional func(status agonesv1.GameServerStatus)) { select { case current := <-state: assert.Equal(t, expected, current.State) additional(current) - case <-time.After(timeout): + case <-time.After(5 * time.Second): assert.Fail(t, "should have gone to Reserved by now") } } assertReservedUntilDuration := func(d time.Duration) func(status agonesv1.GameServerStatus) { return func(status agonesv1.GameServerStatus) { - assert.Equal(t, time.Now().Add(d).Round(time.Second), status.ReservedUntil.Time.Round(time.Second)) + assert.WithinDuration(t, time.Now().Add(d), status.ReservedUntil.Time, 1500*time.Millisecond) } } assertReservedUntilNil := func(status agonesv1.GameServerStatus) { @@ -1033,23 +1055,23 @@ func TestSDKServerReserveTimeout(t *testing.T) { _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: 3}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilDuration(3*time.Second)) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilDuration(3*time.Second)) // Wait for the game server to go back to being Ready. - assertStateChange(agonesv1.GameServerStateRequestReady, 4*time.Second, func(status agonesv1.GameServerStatus) { + assertStateChange(agonesv1.GameServerStateRequestReady, func(status agonesv1.GameServerStatus) { assert.Nil(t, status.ReservedUntil) }) // Test that a 0 second input into Reserved, never will go back to Ready _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: 0}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilNil) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilNil) assert.False(t, sc.reserveTimer.Stop()) // Test that a negative input into Reserved, is the same as a 0 input _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: -100}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilNil) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilNil) assert.False(t, sc.reserveTimer.Stop()) // Test that the timer to move Reserved->Ready is reset when requesting another state. @@ -1057,31 +1079,31 @@ func TestSDKServerReserveTimeout(t *testing.T) { // Test the return to a Ready state. _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: 3}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilDuration(3*time.Second)) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilDuration(3*time.Second)) _, err = sc.Ready(context.Background(), &sdk.Empty{}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateRequestReady, 2*time.Second, assertReservedUntilNil) + assertStateChange(agonesv1.GameServerStateRequestReady, assertReservedUntilNil) assert.False(t, sc.reserveTimer.Stop()) // Test Allocated resets the timer on Reserved->Ready _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: 3}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilDuration(3*time.Second)) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilDuration(3*time.Second)) _, err = sc.Allocate(context.Background(), &sdk.Empty{}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateAllocated, 2*time.Second, assertReservedUntilNil) + assertStateChange(agonesv1.GameServerStateAllocated, assertReservedUntilNil) assert.False(t, sc.reserveTimer.Stop()) // Test Shutdown resets the timer on Reserved->Ready _, err = sc.Reserve(context.Background(), &sdk.Duration{Seconds: 3}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateReserved, 2*time.Second, assertReservedUntilDuration(3*time.Second)) + assertStateChange(agonesv1.GameServerStateReserved, assertReservedUntilDuration(3*time.Second)) _, err = sc.Shutdown(context.Background(), &sdk.Empty{}) assert.NoError(t, err) - assertStateChange(agonesv1.GameServerStateShutdown, 2*time.Second, assertReservedUntilNil) + assertStateChange(agonesv1.GameServerStateShutdown, assertReservedUntilNil) assert.False(t, sc.reserveTimer.Stop()) cancel() @@ -1129,7 +1151,7 @@ func TestSDKServerUpdateCounter(t *testing.T) { updated: true, }, "increment illegal": { - counterName: "widgets", + counterName: "foo", requests: []*beta.UpdateCounterRequest{{ CounterUpdateRequest: &beta.CounterUpdateRequest{ Name: "foo", @@ -1254,31 +1276,32 @@ func TestSDKServerUpdateCounter(t *testing.T) { t.Run(test, func(t *testing.T) { m := agtesting.NewMocks() - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", Generation: 1, - }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - }, - Status: agonesv1.GameServerStatus{ - Counters: counters, + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", Generation: 1, + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + Status: agonesv1.GameServerStatus{ + Counters: counters, + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) updated := make(chan map[string]agonesv1.CounterStatus, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - gs.ObjectMeta.Generation++ - updated <- gs.Status.Counters - return true, gs, nil + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + + updated <- gsCopy.Status.Counters + return true, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -1327,7 +1350,7 @@ func TestSDKServerUpdateCounter(t *testing.T) { agonesv1.CounterStatus{Count: testCase.want.Count, Capacity: testCase.want.Capacity}, value[testCase.counterName]) case <-time.After(10 * time.Second): - assert.Fail(t, "Counter should have been updated") + assert.Fail(t, "Counter should have been patched") } } @@ -1406,31 +1429,32 @@ func TestSDKServerAddListValue(t *testing.T) { t.Run(test, func(t *testing.T) { m := agtesting.NewMocks() - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", Generation: 1, - }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - }, - Status: agonesv1.GameServerStatus{ - Lists: lists, + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", Generation: 1, + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + Status: agonesv1.GameServerStatus{ + Lists: lists, + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) updated := make(chan map[string]agonesv1.ListStatus, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - gs.ObjectMeta.Generation++ - updated <- gs.Status.Lists - return true, gs, nil + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + + updated <- gsCopy.Status.Lists + return true, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -1479,7 +1503,7 @@ func TestSDKServerAddListValue(t *testing.T) { agonesv1.ListStatus{Values: testCase.want.Values, Capacity: testCase.want.Capacity}, value[testCase.listName]) case <-time.After(10 * time.Second): - assert.Fail(t, "List should have been updated") + assert.Fail(t, "List should have been patched") } } @@ -1554,31 +1578,32 @@ func TestSDKServerRemoveListValue(t *testing.T) { t.Run(test, func(t *testing.T) { m := agtesting.NewMocks() - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", Generation: 1, - }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - }, - Status: agonesv1.GameServerStatus{ - Lists: lists, + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", Generation: 1, + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + Status: agonesv1.GameServerStatus{ + Lists: lists, + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) updated := make(chan map[string]agonesv1.ListStatus, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - gs.ObjectMeta.Generation++ - updated <- gs.Status.Lists - return true, gs, nil + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + + updated <- gsCopy.Status.Lists + return true, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -1627,7 +1652,7 @@ func TestSDKServerRemoveListValue(t *testing.T) { agonesv1.ListStatus{Values: testCase.want.Values, Capacity: testCase.want.Capacity}, value[testCase.listName]) case <-time.After(10 * time.Second): - assert.Fail(t, "List should have been updated") + assert.Fail(t, "List should have been patched") } } @@ -1711,31 +1736,32 @@ func TestSDKServerUpdateList(t *testing.T) { t.Run(test, func(t *testing.T) { m := agtesting.NewMocks() - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", Generation: 1, - }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - }, - Status: agonesv1.GameServerStatus{ - Lists: lists, + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", Generation: 1, + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + Status: agonesv1.GameServerStatus{ + Lists: lists, + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) updated := make(chan map[string]agonesv1.ListStatus, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - gs.ObjectMeta.Generation++ - updated <- gs.Status.Lists - return true, gs, nil + + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + + updated <- gsCopy.Status.Lists + return true, gsCopy, nil }) ctx, cancel := context.WithCancel(context.Background()) @@ -1782,7 +1808,7 @@ func TestSDKServerUpdateList(t *testing.T) { agonesv1.ListStatus{Values: testCase.want.Values, Capacity: testCase.want.Capacity}, value[testCase.listName]) case <-time.After(10 * time.Second): - assert.Fail(t, "List should have been updated") + assert.Fail(t, "List should have been patched") } } @@ -1838,30 +1864,32 @@ func TestSDKServerPlayerCapacity(t *testing.T) { sc, err := defaultSidecar(m) require.NoError(t, err) - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - Players: &agonesv1.PlayersSpec{ - InitialCapacity: 10, - }, + Players: &agonesv1.PlayersSpec{ + InitialCapacity: 10, }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) updated := make(chan int64, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - updated <- gs.Status.Players.Capacity - return true, gs, nil + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + + gsCopy := patchGameServer(t, action, &gs) + + updated <- gsCopy.Status.Players.Capacity + return true, gsCopy, nil }) assert.NoError(t, sc.WaitForConnection(ctx)) @@ -1895,7 +1923,7 @@ func TestSDKServerPlayerCapacity(t *testing.T) { case value := <-updated: assert.Equal(t, int64(20), value) case <-time.After(time.Minute): - assert.Fail(t, "Should have been updated") + assert.Fail(t, "Should have been patched") } agtesting.AssertEventContains(t, m.FakeRecorder.Events, "PlayerCapacity Set to 20") @@ -1989,30 +2017,31 @@ func TestSDKServerPlayerConnectAndDisconnect(t *testing.T) { require.NoError(t, err) capacity := int64(3) - m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gs := agonesv1.GameServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", Namespace: "default", + gs := agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", Namespace: "default", ResourceVersion: "0", + }, + Spec: agonesv1.GameServerSpec{ + SdkServer: agonesv1.SdkServer{ + LogLevel: "Debug", }, - Spec: agonesv1.GameServerSpec{ - SdkServer: agonesv1.SdkServer{ - LogLevel: "Debug", - }, - // this is here to give us a reference, so we know when sc.Run() has completed. - Players: &agonesv1.PlayersSpec{ - InitialCapacity: capacity, - }, + // this is here to give us a reference, so we know when sc.Run() has completed. + Players: &agonesv1.PlayersSpec{ + InitialCapacity: capacity, }, - } - gs.ApplyDefaults() - return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil + }, + } + gs.ApplyDefaults() + + m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gs.DeepCopy()}}, nil }) + updated := make(chan *agonesv1.PlayerStatus, 10) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - updated <- gs.Status.Players - return true, gs, nil + m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsCopy := patchGameServer(t, action, &gs) + updated <- gsCopy.Status.Players + return true, gsCopy, nil }) assert.NoError(t, sc.WaitForConnection(ctx)) diff --git a/sdks/go/beta.go b/sdks/go/beta.go index 688f61101e..f9c52ee350 100644 --- a/sdks/go/beta.go +++ b/sdks/go/beta.go @@ -39,8 +39,8 @@ func newBeta(conn *grpc.ClientConn) *Beta { // GetCounterCount returns the Count for a Counter, given the Counter's key (name). // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) GetCounterCount(key string) (int64, error) { - counter, err := a.client.GetCounter(context.Background(), &beta.GetCounterRequest{Name: key}) +func (b *Beta) GetCounterCount(key string) (int64, error) { + counter, err := b.client.GetCounter(context.Background(), &beta.GetCounterRequest{Name: key}) if err != nil { return -1, errors.Wrapf(err, "could not get Counter %s count", key) } @@ -56,11 +56,11 @@ func (a *Beta) GetCounterCount(key string) (int64, error) { // Note: A potential race condition here is that if count values are set from both the SDK and // through the K8s API (Allocation or otherwise), since the SDK append operation back to the CRD // value is batched asynchronous any value incremented past the capacity will be silently truncated. -func (a *Beta) IncrementCounter(key string, amount int64) error { +func (b *Beta) IncrementCounter(key string, amount int64) error { if amount < 0 { return errors.Errorf("amount must be a positive int64, found %d", amount) } - _, err := a.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ + _, err := b.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ CounterUpdateRequest: &beta.CounterUpdateRequest{ Name: key, CountDiff: amount, @@ -74,11 +74,11 @@ func (a *Beta) IncrementCounter(key string, amount int64) error { // DecrementCounter decreases the current count by the given nonnegative integer amount. // The Counter Will not go below 0. Will execute the decrement operation against the current CRD value. // Will error if the count is at 0 (to the latest knowledge of the SDK), and no decrement will occur. -func (a *Beta) DecrementCounter(key string, amount int64) error { +func (b *Beta) DecrementCounter(key string, amount int64) error { if amount < 0 { return errors.Errorf("amount must be a positive int64, found %d", amount) } - _, err := a.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ + _, err := b.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ CounterUpdateRequest: &beta.CounterUpdateRequest{ Name: key, CountDiff: amount * -1, @@ -91,8 +91,8 @@ func (a *Beta) DecrementCounter(key string, amount int64) error { // SetCounterCount sets a count to the given value. Use with care, as this will overwrite any previous // invocations’ value. Cannot be greater than Capacity. -func (a *Beta) SetCounterCount(key string, amount int64) error { - _, err := a.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ +func (b *Beta) SetCounterCount(key string, amount int64) error { + _, err := b.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ CounterUpdateRequest: &beta.CounterUpdateRequest{ Name: key, Count: wrapperspb.Int64(amount), @@ -105,8 +105,8 @@ func (a *Beta) SetCounterCount(key string, amount int64) error { // GetCounterCapacity returns the Capacity for a Counter, given the Counter's key (name). // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) GetCounterCapacity(key string) (int64, error) { - counter, err := a.client.GetCounter(context.Background(), &beta.GetCounterRequest{Name: key}) +func (b *Beta) GetCounterCapacity(key string) (int64, error) { + counter, err := b.client.GetCounter(context.Background(), &beta.GetCounterRequest{Name: key}) if err != nil { return -1, errors.Wrapf(err, "could not get Counter %s capacity", key) } @@ -114,8 +114,8 @@ func (a *Beta) GetCounterCapacity(key string) (int64, error) { } // SetCounterCapacity sets the capacity for the given Counter. A capacity of 0 is no capacity. -func (a *Beta) SetCounterCapacity(key string, amount int64) error { - _, err := a.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ +func (b *Beta) SetCounterCapacity(key string, amount int64) error { + _, err := b.client.UpdateCounter(context.Background(), &beta.UpdateCounterRequest{ CounterUpdateRequest: &beta.CounterUpdateRequest{ Name: key, Capacity: wrapperspb.Int64(amount), @@ -128,8 +128,8 @@ func (a *Beta) SetCounterCapacity(key string, amount int64) error { // GetListCapacity returns the Capacity for a List, given the List's key (name). // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) GetListCapacity(key string) (int64, error) { - list, err := a.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) +func (b *Beta) GetListCapacity(key string) (int64, error) { + list, err := b.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) if err != nil { return -1, errors.Wrapf(err, "could not get List %s", key) } @@ -138,8 +138,8 @@ func (a *Beta) GetListCapacity(key string) (int64, error) { // SetListCapacity sets the capacity for a given list. Capacity must be between 0 and 1000. // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) SetListCapacity(key string, amount int64) error { - _, err := a.client.UpdateList(context.Background(), &beta.UpdateListRequest{ +func (b *Beta) SetListCapacity(key string, amount int64) error { + _, err := b.client.UpdateList(context.Background(), &beta.UpdateListRequest{ List: &beta.List{ Name: key, Capacity: amount, @@ -155,8 +155,8 @@ func (a *Beta) SetListCapacity(key string, amount int64) error { // ListContains returns if a string exists in a List's values list, given the List's key (name) // and the string value. Search is case-sensitive. // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) ListContains(key, value string) (bool, error) { - list, err := a.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) +func (b *Beta) ListContains(key, value string) (bool, error) { + list, err := b.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) if err != nil { return false, errors.Wrapf(err, "could not get List %s", key) } @@ -170,8 +170,8 @@ func (a *Beta) ListContains(key, value string) (bool, error) { // GetListLength returns the length of the Values list for a List, given the List's key (name). // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) GetListLength(key string) (int, error) { - list, err := a.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) +func (b *Beta) GetListLength(key string) (int, error) { + list, err := b.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) if err != nil { return -1, errors.Wrapf(err, "could not get List %s", key) } @@ -180,8 +180,8 @@ func (a *Beta) GetListLength(key string) (int, error) { // GetListValues returns the Values for a List, given the List's key (name). // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) GetListValues(key string) ([]string, error) { - list, err := a.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) +func (b *Beta) GetListValues(key string) ([]string, error) { + list, err := b.client.GetList(context.Background(), &beta.GetListRequest{Name: key}) if err != nil { return nil, errors.Wrapf(err, "could not get List %s", key) } @@ -191,8 +191,8 @@ func (a *Beta) GetListValues(key string) ([]string, error) { // AppendListValue appends a string to a List's values list, given the List's key (name) // and the string value. Will error if the string already exists in the list. // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) AppendListValue(key, value string) error { - _, err := a.client.AddListValue(context.Background(), &beta.AddListValueRequest{Name: key, Value: value}) +func (b *Beta) AppendListValue(key, value string) error { + _, err := b.client.AddListValue(context.Background(), &beta.AddListValueRequest{Name: key, Value: value}) if err != nil { return errors.Wrapf(err, "could not get List %s", key) } @@ -202,8 +202,8 @@ func (a *Beta) AppendListValue(key, value string) error { // DeleteListValue removes a string from a List's values list, given the List's key (name) // and the string value. Will error if the string does not exist in the list. // Will error if the key was not predefined in the GameServer resource on creation. -func (a *Beta) DeleteListValue(key, value string) error { - _, err := a.client.RemoveListValue(context.Background(), &beta.RemoveListValueRequest{Name: key, Value: value}) +func (b *Beta) DeleteListValue(key, value string) error { + _, err := b.client.RemoveListValue(context.Background(), &beta.RemoveListValueRequest{Name: key, Value: value}) if err != nil { return errors.Wrapf(err, "could not get List %s", key) } diff --git a/sdks/go/beta_test.go b/sdks/go/beta_test.go index 5c13a875c7..d651ca31c1 100644 --- a/sdks/go/beta_test.go +++ b/sdks/go/beta_test.go @@ -62,104 +62,104 @@ func TestBetaGetAndUpdateCounter(t *testing.T) { Capacity: 500, } - a := Beta{ + b := Beta{ client: mock, } t.Parallel() t.Run("Set Counter and Set Capacity", func(t *testing.T) { - count, err := a.GetCounterCount("sessions") + count, err := b.GetCounterCount("sessions") assert.NoError(t, err) assert.Equal(t, sessions.Count, count) - capacity, err := a.GetCounterCapacity("sessions") + capacity, err := b.GetCounterCapacity("sessions") assert.NoError(t, err) assert.Equal(t, sessions.Capacity, capacity) wantCapacity := int64(25) - err = a.SetCounterCapacity("sessions", wantCapacity) + err = b.SetCounterCapacity("sessions", wantCapacity) assert.NoError(t, err) - capacity, err = a.GetCounterCapacity("sessions") + capacity, err = b.GetCounterCapacity("sessions") assert.NoError(t, err) assert.Equal(t, wantCapacity, capacity) wantCount := int64(10) - err = a.SetCounterCount("sessions", wantCount) + err = b.SetCounterCount("sessions", wantCount) assert.NoError(t, err) - count, err = a.GetCounterCount("sessions") + count, err = b.GetCounterCount("sessions") assert.NoError(t, err) assert.Equal(t, wantCount, count) }) t.Run("Get and Set Non-Defined Counter", func(t *testing.T) { - _, err := a.GetCounterCount("secessions") + _, err := b.GetCounterCount("secessions") assert.Error(t, err) - _, err = a.GetCounterCapacity("secessions") + _, err = b.GetCounterCapacity("secessions") assert.Error(t, err) - err = a.SetCounterCapacity("secessions", int64(100)) + err = b.SetCounterCapacity("secessions", int64(100)) assert.Error(t, err) - err = a.SetCounterCount("secessions", int64(0)) + err = b.SetCounterCount("secessions", int64(0)) assert.Error(t, err) }) // nolint:dupl // testing DecrementCounter and IncrementCounter are not duplicates. t.Run("Decrement Counter Fails then Success", func(t *testing.T) { - count, err := a.GetCounterCount("games") + count, err := b.GetCounterCount("games") assert.NoError(t, err) assert.Equal(t, games.Count, count) - err = a.DecrementCounter("games", 21) + err = b.DecrementCounter("games", 21) assert.Error(t, err) - count, err = a.GetCounterCount("games") + count, err = b.GetCounterCount("games") assert.NoError(t, err) assert.Equal(t, games.Count, count) - err = a.DecrementCounter("games", -12) + err = b.DecrementCounter("games", -12) assert.Error(t, err) - count, err = a.GetCounterCount("games") + count, err = b.GetCounterCount("games") assert.NoError(t, err) assert.Equal(t, games.Count, count) - err = a.DecrementCounter("games", 12) + err = b.DecrementCounter("games", 12) assert.NoError(t, err) - count, err = a.GetCounterCount("games") + count, err = b.GetCounterCount("games") assert.NoError(t, err) assert.Equal(t, int64(0), count) }) // nolint:dupl // testing DecrementCounter and IncrementCounter are not duplicates. t.Run("Increment Counter Fails then Success", func(t *testing.T) { - count, err := a.GetCounterCount("gamers") + count, err := b.GetCounterCount("gamers") assert.NoError(t, err) assert.Equal(t, gamers.Count, count) - err = a.IncrementCounter("gamers", 250) + err = b.IncrementCounter("gamers", 250) assert.Error(t, err) - count, err = a.GetCounterCount("gamers") + count, err = b.GetCounterCount("gamers") assert.NoError(t, err) assert.Equal(t, gamers.Count, count) - err = a.IncrementCounter("gamers", -237) + err = b.IncrementCounter("gamers", -237) assert.Error(t, err) - count, err = a.GetCounterCount("gamers") + count, err = b.GetCounterCount("gamers") assert.NoError(t, err) assert.Equal(t, gamers.Count, count) - err = a.IncrementCounter("gamers", 237) + err = b.IncrementCounter("gamers", 237) assert.NoError(t, err) - count, err = a.GetCounterCount("gamers") + count, err = b.GetCounterCount("gamers") assert.NoError(t, err) assert.Equal(t, int64(500), count) }) @@ -203,74 +203,74 @@ func TestBetaGetAndUpdateList(t *testing.T) { Capacity: 5, } - a := Beta{ + b := Beta{ client: mock, } t.Parallel() t.Run("Get and Set List Capacity", func(t *testing.T) { - capacity, err := a.GetListCapacity("foo") + capacity, err := b.GetListCapacity("foo") assert.NoError(t, err) assert.Equal(t, foo.Capacity, capacity) wantCapacity := int64(5) - err = a.SetListCapacity("foo", wantCapacity) + err = b.SetListCapacity("foo", wantCapacity) assert.NoError(t, err) - capacity, err = a.GetListCapacity("foo") + capacity, err = b.GetListCapacity("foo") assert.NoError(t, err) assert.Equal(t, wantCapacity, capacity) }) t.Run("Get List Length, Get List Values, ListContains, and Append List Value", func(t *testing.T) { - length, err := a.GetListLength("bar") + length, err := b.GetListLength("bar") assert.NoError(t, err) assert.Equal(t, len(bar.Values), length) - values, err := a.GetListValues("bar") + values, err := b.GetListValues("bar") assert.NoError(t, err) assert.Equal(t, bar.Values, values) - err = a.AppendListValue("bar", "ghi") + err = b.AppendListValue("bar", "ghi") assert.NoError(t, err) - length, err = a.GetListLength("bar") + length, err = b.GetListLength("bar") assert.NoError(t, err) assert.Equal(t, len(bar.Values)+1, length) wantValues := []string{"abc", "def", "ghi"} - values, err = a.GetListValues("bar") + values, err = b.GetListValues("bar") assert.NoError(t, err) assert.Equal(t, wantValues, values) - contains, err := a.ListContains("bar", "ghi") + contains, err := b.ListContains("bar", "ghi") assert.NoError(t, err) assert.True(t, contains) }) t.Run("Get List Length, Get List Values, ListContains, and Delete List Value", func(t *testing.T) { - length, err := a.GetListLength("baz") + length, err := b.GetListLength("baz") assert.NoError(t, err) assert.Equal(t, len(baz.Values), length) - values, err := a.GetListValues("baz") + values, err := b.GetListValues("baz") assert.NoError(t, err) assert.Equal(t, baz.Values, values) - err = a.DeleteListValue("baz", "456") + err = b.DeleteListValue("baz", "456") assert.NoError(t, err) - length, err = a.GetListLength("baz") + length, err = b.GetListLength("baz") assert.NoError(t, err) assert.Equal(t, len(baz.Values)-1, length) wantValues := []string{"123", "789"} - values, err = a.GetListValues("baz") + values, err = b.GetListValues("baz") assert.NoError(t, err) assert.Equal(t, wantValues, values) - contains, err := a.ListContains("baz", "456") + contains, err := b.ListContains("baz", "456") assert.NoError(t, err) assert.False(t, contains) }) @@ -282,15 +282,15 @@ type betaMock struct { lists map[string]*beta.List } -func (a *betaMock) GetCounter(ctx context.Context, in *beta.GetCounterRequest, opts ...grpc.CallOption) (*beta.Counter, error) { - if counter, ok := a.counters[in.Name]; ok { +func (b *betaMock) GetCounter(ctx context.Context, in *beta.GetCounterRequest, opts ...grpc.CallOption) (*beta.Counter, error) { + if counter, ok := b.counters[in.Name]; ok { return counter, nil } return nil, errors.Errorf("counter not found: %s", in.Name) } -func (a *betaMock) UpdateCounter(ctx context.Context, in *beta.UpdateCounterRequest, opts ...grpc.CallOption) (*beta.Counter, error) { - counter, err := a.GetCounter(ctx, &beta.GetCounterRequest{Name: in.CounterUpdateRequest.Name}) +func (b *betaMock) UpdateCounter(ctx context.Context, in *beta.UpdateCounterRequest, opts ...grpc.CallOption) (*beta.Counter, error) { + counter, err := b.GetCounter(ctx, &beta.GetCounterRequest{Name: in.CounterUpdateRequest.Name}) if err != nil { return nil, err } @@ -319,17 +319,17 @@ func (a *betaMock) UpdateCounter(ctx context.Context, in *beta.UpdateCounterRequ in.CounterUpdateRequest) } - a.counters[in.CounterUpdateRequest.Name] = counter - return a.counters[in.CounterUpdateRequest.Name], nil + b.counters[in.CounterUpdateRequest.Name] = counter + return b.counters[in.CounterUpdateRequest.Name], nil } // GetList returns the list of betaMock. Note: unlike the SDK Server, this does not return // a list with any pending batched changes applied. -func (a *betaMock) GetList(ctx context.Context, in *beta.GetListRequest, opts ...grpc.CallOption) (*beta.List, error) { +func (b *betaMock) GetList(ctx context.Context, in *beta.GetListRequest, opts ...grpc.CallOption) (*beta.List, error) { if in == nil { return nil, errors.Errorf("GetListRequest cannot be nil") } - if list, ok := a.lists[in.Name]; ok { + if list, ok := b.lists[in.Name]; ok { return list, nil } return nil, errors.Errorf("list not found: %s", in.Name) @@ -337,11 +337,11 @@ func (a *betaMock) GetList(ctx context.Context, in *beta.GetListRequest, opts .. // Note: unlike the SDK Server, UpdateList does not batch changes and instead updates the list // directly. -func (a *betaMock) UpdateList(ctx context.Context, in *beta.UpdateListRequest, opts ...grpc.CallOption) (*beta.List, error) { +func (b *betaMock) UpdateList(ctx context.Context, in *beta.UpdateListRequest, opts ...grpc.CallOption) (*beta.List, error) { if in == nil { return nil, errors.Errorf("UpdateListRequest cannot be nil") } - list, ok := a.lists[in.List.Name] + list, ok := b.lists[in.List.Name] if !ok { return nil, errors.Errorf("list not found: %s", in.List.Name) } @@ -352,17 +352,17 @@ func (a *betaMock) UpdateList(ctx context.Context, in *beta.UpdateListRequest, o if len(list.Values) > int(list.Capacity) { list.Values = append([]string{}, list.Values[:list.Capacity]...) } - a.lists[in.List.Name] = list + b.lists[in.List.Name] = list return &beta.List{}, nil } // Note: unlike the SDK Server, AddListValue does not batch changes and instead updates the list // directly. -func (a *betaMock) AddListValue(ctx context.Context, in *beta.AddListValueRequest, opts ...grpc.CallOption) (*beta.List, error) { +func (b *betaMock) AddListValue(ctx context.Context, in *beta.AddListValueRequest, opts ...grpc.CallOption) (*beta.List, error) { if in == nil { return nil, errors.Errorf("AddListValueRequest cannot be nil") } - list, ok := a.lists[in.Name] + list, ok := b.lists[in.Name] if !ok { return nil, errors.Errorf("list not found: %s", in.Name) } @@ -375,17 +375,17 @@ func (a *betaMock) AddListValue(ctx context.Context, in *beta.AddListValueReques } } list.Values = append(list.Values, in.Value) - a.lists[in.Name] = list + b.lists[in.Name] = list return &beta.List{}, nil } // Note: unlike the SDK Server, RemoveListValue does not batch changes and instead updates the list // directly. -func (a *betaMock) RemoveListValue(ctx context.Context, in *beta.RemoveListValueRequest, opts ...grpc.CallOption) (*beta.List, error) { +func (b *betaMock) RemoveListValue(ctx context.Context, in *beta.RemoveListValueRequest, opts ...grpc.CallOption) (*beta.List, error) { if in == nil { return nil, errors.Errorf("RemoveListValueRequest cannot be nil") } - list, ok := a.lists[in.Name] + list, ok := b.lists[in.Name] if !ok { return nil, errors.Errorf("list not found: %s", in.Name) } @@ -394,7 +394,7 @@ func (a *betaMock) RemoveListValue(ctx context.Context, in *beta.RemoveListValue continue } list.Values = append(list.Values[:i], list.Values[i+1:]...) - a.lists[in.Name] = list + b.lists[in.Name] = list return &beta.List{}, nil } return nil, errors.Errorf("not found. Value: %s not found in List: %s", in.Value, in.Name) diff --git a/test/e2e/gameserver_test.go b/test/e2e/gameserver_test.go index cc2cf32a5b..87b5874a72 100644 --- a/test/e2e/gameserver_test.go +++ b/test/e2e/gameserver_test.go @@ -34,6 +34,7 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/wait" @@ -1711,3 +1712,61 @@ func TestGameServerSlowStart(t *testing.T) { _, err := framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs) assert.NoError(t, err) } + +func TestGameServerPatch(t *testing.T) { + if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + t.SkipNow() + } + t.Parallel() + ctx := context.Background() + + gs := framework.DefaultGameServer(framework.Namespace) + gs, err := framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs) + require.NoError(t, err) + defer framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Delete(ctx, gs.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint: errcheck + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + + // Create a gameserver to patch against + gsCopy := gs.DeepCopy() + gsCopy.ObjectMeta.Labels = map[string]string{"foo": "foo-value"} + + patch, err := gs.Patch(gsCopy) + require.NoError(t, err) + patchString := string(patch) + require.Contains(t, patchString, fmt.Sprintf("{\"op\":\"test\",\"path\":\"/metadata/resourceVersion\",\"value\":%q}", gs.ObjectMeta.ResourceVersion)) + require.Contains(t, patchString, "{\"op\":\"add\",\"path\":\"/metadata/labels\",\"value\":{\"foo\":\"foo-value\"}}") + + // Confirm patch is applied correctly + patchedGs, err := framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Patch(ctx, gs.GetObjectMeta().GetName(), types.JSONPatchType, patch, metav1.PatchOptions{}) + require.NoError(t, err) + require.Equal(t, patchedGs.ObjectMeta.Labels, map[string]string{"foo": "foo-value"}) + require.NotEqual(t, patchedGs.ObjectMeta.ResourceVersion, gs.ObjectMeta.ResourceVersion) + + // Confirm a patch applied to an old version of a game server is not applied + gsCopy.ObjectMeta.Labels = map[string]string{"bar": "bar-value"} + patch, err = gs.Patch(gsCopy) + require.NoError(t, err) + + _, err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Patch(ctx, gs.GetObjectMeta().GetName(), types.JSONPatchType, patch, metav1.PatchOptions{}) + require.Error(t, err) + + getGs, err := framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Get(ctx, gs.ObjectMeta.Name, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, getGs.ObjectMeta.Labels, map[string]string{"foo": "foo-value"}) + require.Equal(t, getGs.ObjectMeta.ResourceVersion, patchedGs.ObjectMeta.ResourceVersion) + + // Confirm patch goes through with the most up-to-date game server + gsCopy = patchedGs.DeepCopy() + gsCopy.ObjectMeta.Labels = map[string]string{"bar": "bar-value"} + patch, err = patchedGs.Patch(gsCopy) + require.NoError(t, err) + + rePatchedGs, err := framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Patch(ctx, gs.GetObjectMeta().GetName(), types.JSONPatchType, patch, metav1.PatchOptions{}) + require.NoError(t, err) + require.Equal(t, rePatchedGs.ObjectMeta.Labels, map[string]string{"bar": "bar-value"}) + + getGs, err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Get(ctx, gs.ObjectMeta.Name, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, getGs.ObjectMeta.Labels, map[string]string{"bar": "bar-value"}) + require.Equal(t, getGs.ObjectMeta.ResourceVersion, rePatchedGs.ObjectMeta.ResourceVersion) +} From 8749377c763dc78e21eea26f2f29130cea087b8d Mon Sep 17 00:00:00 2001 From: Kalaiselvim <117940852+Kalaiselvi84@users.noreply.github.com> Date: Thu, 9 May 2024 18:34:08 +0000 Subject: [PATCH 2/5] Set Minimum Buffer Size to 1 (#3749) * Set Minimums for DesiredCapacity and Buffer to 1 * fix lint * Fix TestApplyListPolicy * review changes * Prevent scaling down to zero replicas in scaleDown logic * modify TestApplyCounterPolicy * remove number of replicas check * modified scaleDown method and used helper function * suggested changes --------- Co-authored-by: Mark Mandel --- pkg/fleetautoscalers/fleetautoscalers.go | 13 ++++++++++--- pkg/fleetautoscalers/fleetautoscalers_test.go | 15 ++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/fleetautoscalers/fleetautoscalers.go b/pkg/fleetautoscalers/fleetautoscalers.go index f1df1d8bbf..5ef77e0edf 100644 --- a/pkg/fleetautoscalers/fleetautoscalers.go +++ b/pkg/fleetautoscalers/fleetautoscalers.go @@ -28,6 +28,10 @@ import ( "strings" "time" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/uuid" + agonesv1 "agones.dev/agones/pkg/apis/agones/v1" autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" listeragonesv1 "agones.dev/agones/pkg/client/listers/agones/v1" @@ -35,9 +39,6 @@ import ( "agones.dev/agones/pkg/gameservers" gssets "agones.dev/agones/pkg/gameserversets" "agones.dev/agones/pkg/util/runtime" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/apimachinery/pkg/util/uuid" ) var tlsConfig = &tls.Config{} @@ -319,6 +320,12 @@ func applyCounterOrListPolicy(c *autoscalingv1.CounterPolicy, l *autoscalingv1.L if err != nil { return 0, false, err } + // If the Aggregated Allocated Counts is 0 then desired capacity gets calculated as 0. If the + // capacity of 1 replica is equal to or greater than minimum capacity we can exit early. + if aggAllocatedCount <= 0 && capacity >= minCapacity { + return 1, true, nil + } + // The desired TOTAL capacity based on the Aggregated Allocated Counts (see applyBufferPolicy for explanation) desiredCapacity := int64(math.Ceil(float64(aggAllocatedCount*100) / float64(100-bufferPercent))) // Convert into a desired AVAILABLE capacity aka the buffer diff --git a/pkg/fleetautoscalers/fleetautoscalers_test.go b/pkg/fleetautoscalers/fleetautoscalers_test.go index 175971f660..0b1f9bee74 100644 --- a/pkg/fleetautoscalers/fleetautoscalers_test.go +++ b/pkg/fleetautoscalers/fleetautoscalers_test.go @@ -24,17 +24,18 @@ import ( "net/http/httptest" "testing" - agonesv1 "agones.dev/agones/pkg/apis/agones/v1" - autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" - "agones.dev/agones/pkg/gameservers" - agtesting "agones.dev/agones/pkg/testing" - utilruntime "agones.dev/agones/pkg/util/runtime" "github.com/stretchr/testify/assert" admregv1 "k8s.io/api/admissionregistration/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" k8stesting "k8s.io/client-go/testing" + + agonesv1 "agones.dev/agones/pkg/apis/agones/v1" + autoscalingv1 "agones.dev/agones/pkg/apis/autoscaling/v1" + "agones.dev/agones/pkg/gameservers" + agtesting "agones.dev/agones/pkg/testing" + utilruntime "agones.dev/agones/pkg/util/runtime" ) const ( @@ -1950,8 +1951,8 @@ func TestApplyListPolicy(t *testing.T) { }}}}, }, want: expected{ - replicas: 0, - limited: false, + replicas: 1, + limited: true, wantErr: false, }, }, From 39d6e1fb420499bccabb3df8fc0fd96fbb1b7d14 Mon Sep 17 00:00:00 2001 From: Vicente Ferrara <47219931+vicentefb@users.noreply.github.com> Date: Thu, 9 May 2024 15:42:37 -0700 Subject: [PATCH 3/5] disable FeatureAutopilotPassthroughPort dev feature (#3815) --- install/helm/agones/defaultfeaturegates.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/helm/agones/defaultfeaturegates.yaml b/install/helm/agones/defaultfeaturegates.yaml index 6ec8b01c29..e78863c81a 100644 --- a/install/helm/agones/defaultfeaturegates.yaml +++ b/install/helm/agones/defaultfeaturegates.yaml @@ -26,7 +26,7 @@ RollingUpdateFix: false PortRanges: false # Dev features -FeatureAutopilotPassthroughPort: true +FeatureAutopilotPassthroughPort: false # Example feature Example: false From 67f23dff3761add24bbf8012749303e260239ae4 Mon Sep 17 00:00:00 2001 From: Vicente Ferrara <47219931+vicentefb@users.noreply.github.com> Date: Thu, 9 May 2024 17:01:17 -0700 Subject: [PATCH 4/5] disable dev feature (#3816) --- pkg/util/runtime/features.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/runtime/features.go b/pkg/util/runtime/features.go index 38be5ff5cb..ceafc52323 100644 --- a/pkg/util/runtime/features.go +++ b/pkg/util/runtime/features.go @@ -120,7 +120,7 @@ var ( FeaturePortRanges: false, // Dev features - FeatureAutopilotPassthroughPort: true, + FeatureAutopilotPassthroughPort: false, // Example feature FeatureExample: false, From 1b06e945ec086d14ceffdb02ef818b768e206b6a Mon Sep 17 00:00:00 2001 From: igooch Date: Fri, 10 May 2024 20:29:03 -0700 Subject: [PATCH 5/5] SDK proto compatibility guarantees and deprecation policies documentation (#3774) * SDK proto compatibility guarantees and deprecation policies. * Minor revisions per reviewer comments * Moves SDK proto compatibity and deprecation policy documentation to the upgrades page * Small wording changes * Additional CUJ type questions to the upgrades path * Changes per reviewer comments --- .../content/en/docs/Installation/upgrading.md | 100 ++++++++++++++++-- 1 file changed, 93 insertions(+), 7 deletions(-) diff --git a/site/content/en/docs/Installation/upgrading.md b/site/content/en/docs/Installation/upgrading.md index bc529b3659..db381ae27f 100644 --- a/site/content/en/docs/Installation/upgrading.md +++ b/site/content/en/docs/Installation/upgrading.md @@ -8,13 +8,13 @@ description: > --- {{< alert color="info" title="Note" >}} -Whichever approach you take to upgrading Agones, make sure to test it in your development environment +Whichever approach you take to upgrading Agones, make sure to test it in your development environment before applying it to production. {{< /alert >}} ## Upgrading Agones -The following are strategies for safely upgrading Agones from one version to another. They may require adjustment to +The following are strategies for safely upgrading Agones from one version to another. They may require adjustment to your particular game architecture but should provide a solid foundation for updating Agones safely. The recommended approach is to use [multiple clusters](#upgrading-agones-multiple-clusters), such that the upgrade can be tested @@ -23,14 +23,14 @@ gradually with production load and easily rolled back if the need arises. {{< alert color="warning" title="Warning" >}} Changing [Feature Gates]({{% ref "/docs/Guides/feature-stages.md#feature-gates" %}}) within your Agones install can constitute an "upgrade" as it may create or remove functionality -in the Agones installation that may not be forward or backward compatible with installed resources in an existing +in the Agones installation that may not be forward or backward compatible with installed resources in an existing installation. {{< /alert >}} ### Upgrading Agones: Multiple Clusters We essentially want to transition our GameServer allocations from a cluster with the old version of Agones, -to a cluster with the upgraded version of Agones while ensuring nothing surprising +to a cluster with the upgraded version of Agones while ensuring nothing surprising happens during this process. This also allows easy rollback to the previous infrastructure that we already know to be working in production, with @@ -94,14 +94,14 @@ gradually with production load and easily rolled back if the need arises. Agones has [multiple supported Kubernetes versions]({{< relref "_index.md#agones-and-kubernetes-supported-versions" >}}) for each version. You can stick with a minor Kubernetes version until it is not supported by Agones, but it is recommended to do supported minor (e.g. 1.12.1 ➡ 1.13.2) Kubernetes version upgrades at the same time as a matching Agones upgrades. -Patch upgrades (e.g. 1.12.1 ➡ 1.12.3) within the same minor version of Kubernetes can be done at any time. +Patch upgrades (e.g. 1.12.1 ➡ 1.12.3) within the same minor version of Kubernetes can be done at any time. ### Multiple Clusters This process is very similar to the [Upgrading Agones: Multiple Clusters](#upgrading-agones-multiple-clusters) approach above. We essentially want to transition our GameServer allocations from a cluster with the old version of Kubernetes, -to a cluster with the upgraded version of Kubernetes while ensuring nothing surprising +to a cluster with the upgraded version of Kubernetes while ensuring nothing surprising happens during this process. This also allows easy rollback to the previous infrastructure that we already know to be working in production, with @@ -128,7 +128,93 @@ upgrades. between the Agones controller being recreated and GameServers being deleted doesn't occur, and GameServers can end up stuck in erroneous states. 1. Start and complete your control plane upgrade(s). 1. Start and complete your node upgrades. -1. Scale your Fleets back up and/or recreate your GameServers. +1. Scale your Fleets back up and/or recreate your GameServers. 1. Run any other tests to ensure the Agones installation is still working as expected. 1. Close your maintenance window. 7. Congratulations - you have now upgraded to a new version of Kubernetes! 👍 + +{{% feature publishVersion="1.41.0" %}} +## SDK Compatibility Guarantees + +The SDK compatibility contract aims to ensure smooth upgrades and reduce the need for frequent +binary redeployments for game server users due to SDK changes. We implement SDK feature versioning +that matches our [Feature Gates]({{% ref "/docs/Guides/feature-stages.md#feature-gates" %}}) to +provide clear documentation of API maturity levels and history (Stable, Beta, Alpha) with release +information. + +**Our SDK Server compatibility contract as of Agones v1.41.0**: A +game server binary using Beta and Stable SDKs will remain compatible with a _newer_ Agones Release, +within possible deprecation windows: +- If your game server uses a non-deprecated Stable API, your binary will be compatible for 10 +releases (~1y) or more starting from the SDK version packaged. + - For example, if the game server uses non-deprecated stable APIs in the 1.40 SDK, it will be + compatible through the 1.50 SDK. + - Stable APIs will almost certainly be compatible beyond 10 releases, as deprecation of stable + APIs is rare, but 10 releases is guaranteed. +- If your game server uses a non-deprecated Beta API, your binary will be guaranteed compatible for + 5 releases (~6mo), but may be compatible past that point. +- Alpha SDK APIs are subject to change between releases. + - A game server binary using Alpha SDKs may not be compatible with a newer Agones release if + breaking changes have been made between releases. + - When we make incompatible Alpha changes, we will document the APIs involved. + +## SDK Deprecation Policies as of Agones + +- Client SDK updates are not mandatory for game server binaries except for SDK when the underlying +proto format has deprecations or breaking Alpha API changes. + +- Breaking changes will be called out in upgrade documentation to allow admins to plan their upgrades. + +- Expect to check if there are breaking changes to Stable APIs you use yearly or Beta APIs semi-annually. + +### Stable Deprecation Policies + +A Stable API may be marked as deprecated in release X and removed from Stable in release X+10. + +### Beta Deprecation Policies + +When a SDK API feature graduates from Beta to Stable at release X, the API will be present in both Beta and +Stable surfaces from release X to release X+5. The Beta API is marked as deprecated in release X and +removed from Beta in release X+5. +A Beta API may be marked as deprecated in release X and removed from Beta in release X+5 without the +API graduating to Stable if it's determined that changes need to be made to the Beta API. + +### Alpha Deprecation Policies + +There is no guaranteed compatibility between releases for Alpha SDKs. When an Alpha API +graduates to Beta the API will be deleted from the Alpha SDK with no overlapping release. +An API may be removed from the Alpha SDK during any release without graduating to Beta. + +## SDK APIs and Stability Levels + +"Legacy" indicates that this API has been in the SDK Server in a release before we began tracking +SDK compatibility with release 1.41.0. \ +The Actions may differ from the [Client SDK]({{< relref "Client SDKs">}}) depending on how each +Client SDK is implemented. + +| Area | Action | Stable | Beta | Alpha | +|---------------------|-----------------------|--------|------|--------| +| Lifecycle | Ready | Legacy | | | +| Lifecycle | Health | Legacy | | | +| Lifecycle | Reserve | Legacy | | | +| Lifecycle | Allocate | Legacy | | | +| Lifecycle | Shutdown | Legacy | | | +| Configuration | GetGameServer | Legacy | | | +| Configuration | WatchGameServer | Legacy | | | +| Metadata | SetAnnotation | Legacy | | | +| Metadata | SetLabel | Legacy | | | +| Counters | GetCounter | | | 1.37.0 | +| Counters | UpdateCounter | | | 1.37.0 | +| Lists | GetList | | | 1.37.0 | +| Lists | UpdateList | | | 1.37.0 | +| Lists | AddListValue | | | 1.37.0 | +| Lists | RemoveListValue | | | 1.37.0 | +| Player Tracking | GetPlayerCapacity | | | Legacy | +| Player Tracking | SetPlayerCapacity | | | Legacy | +| Player Tracking | PlayerConnect | | | Legacy | +| Player Tracking | GetConnectedPlayers | | | Legacy | +| Player Tracking | IsPlayerConnected | | | Legacy | +| Player Tracking | GetPlayerCount | | | Legacy | +| Player Tracking | PlayerDisconnect | | | Legacy | + +{{% /feature %}}