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) +}