From ae8c71449a940584d5b9da2a3f199a51fc2b3ca2 Mon Sep 17 00:00:00 2001 From: igooch Date: Wed, 10 May 2023 16:27:54 -0700 Subject: [PATCH] GameServerAllocation Actions for Counters and Lists (#3117) * Counts and Lists Actions on Game Server Allocation * Adds test for findGameServerForAllocation * Updates ListAction field to avoid pointer to a list * Generated Files * Moves counter and list action kick to the Allocator.goAdds logging of any counter or list actions as warning events * Updates actions to return single error * Move event recording after successful game server update --- pkg/apis/agones/v1/common.go | 6 +- pkg/apis/agones/v1/gameserver.go | 99 ++++ pkg/apis/agones/v1/gameserver_test.go | 452 ++++++++++++++++++ .../allocation/v1/gameserverallocation.go | 63 +++ .../v1/gameserverallocation_test.go | 225 +++++++++ .../allocation/v1/zz_generated.deepcopy.go | 71 +++ pkg/gameserverallocations/allocator.go | 38 +- pkg/gameserverallocations/allocator_test.go | 156 +++++- .../Reference/agones_crd_api_reference.html | 147 ++++++ 9 files changed, 1248 insertions(+), 9 deletions(-) diff --git a/pkg/apis/agones/v1/common.go b/pkg/apis/agones/v1/common.go index f1a5539dc4..12fcd3d212 100644 --- a/pkg/apis/agones/v1/common.go +++ b/pkg/apis/agones/v1/common.go @@ -26,7 +26,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) -// Block of const Error messages +// Block of const Error messages and GameServerAllocation Counter actions const ( ErrContainerRequired = "Container is required when using multiple containers in the pod template" ErrHostPort = "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy" @@ -34,6 +34,10 @@ const ( ErrContainerPortRequired = "ContainerPort must be defined for Dynamic and Static PortPolicies" ErrContainerPortPassthrough = "ContainerPort cannot be specified with Passthrough PortPolicy" ErrContainerNameInvalid = "Container must be empty or the name of a container in the pod template" + // GameServerAllocationIncrement is a Counter Action that indiciates the Counter's Count should be incremented at Allocation. + GameServerAllocationIncrement string = "Increment" + // GameServerAllocationDecrement is a Counter Action that indiciates the Counter's Count should be decremented at Allocation. + GameServerAllocationDecrement string = "Decrement" ) // AggregatedPlayerStatus stores total player tracking values diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index 392b250e3f..c300a3bd55 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -926,3 +926,102 @@ func (gs *GameServer) Patch(delta *GameServer) ([]byte, error) { result, err = json.Marshal(patch) return result, errors.Wrapf(err, "error creating json for patch for GameServer %s", gs.ObjectMeta.Name) } + +// UpdateCount increments or decrements a CounterStatus on a Game Server by the given amount. +func (gs *GameServer) UpdateCount(name string, action string, amount int64) error { + if !(action == GameServerAllocationIncrement || action == GameServerAllocationDecrement) { + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Allocation action must be one of %s or %s", name, action, amount, GameServerAllocationIncrement, GameServerAllocationDecrement) + } + if amount < 0 { + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Amount must be greater than 0", name, action, amount) + } + if counter, ok := gs.Status.Counters[name]; ok { + cnt := counter.Count + if action == GameServerAllocationIncrement { + cnt += amount + // only check for Count > Capacity when incrementing + if cnt > counter.Capacity { + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Incremented Count %d is greater than available capacity %d", name, action, amount, cnt, counter.Capacity) + } + } else { + cnt -= amount + // only check for Count < 0 when decrementing + if cnt < 0 { + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Decremented Count %d is less than 0", name, action, amount, cnt) + } + } + counter.Count = cnt + gs.Status.Counters[name] = counter + return nil + } + return errors.Errorf("unable to UpdateCount with Name %s, Action %s, Amount %d. Counter not found in GameServer %s", name, action, amount, gs.ObjectMeta.GetName()) +} + +// UpdateCounterCapacity updates the CounterStatus Capacity to the given capacity. +func (gs *GameServer) UpdateCounterCapacity(name string, capacity int64) error { + if capacity < 0 { + return errors.Errorf("unable to UpdateCounterCapacity: Name %s, Capacity %d. Capacity must be greater than or equal to 0", name, capacity) + } + if counter, ok := gs.Status.Counters[name]; ok { + counter.Capacity = capacity + gs.Status.Counters[name] = counter + return nil + } + return errors.Errorf("unable to UpdateCounterCapacity: Name %s, Capacity %d. Counter not found in GameServer %s", name, capacity, gs.ObjectMeta.GetName()) +} + +// UpdateListCapacity updates the ListStatus Capacity to the given capacity. +func (gs *GameServer) UpdateListCapacity(name string, capacity int64) error { + if capacity < 0 || capacity > 1000 { + return errors.Errorf("unable to UpdateListCapacity: Name %s, Capacity %d. Capacity must be between 0 and 1000, inclusive", name, capacity) + } + if list, ok := gs.Status.Lists[name]; ok { + list.Capacity = capacity + gs.Status.Lists[name] = list + return nil + } + return errors.Errorf("unable to UpdateListCapacity: Name %s, Capacity %d. List not found in GameServer %s", name, capacity, gs.ObjectMeta.GetName()) +} + +// AppendListValues adds unique values to the ListStatus Values list. +func (gs *GameServer) AppendListValues(name string, values []string) error { + if len(values) == 0 { + return errors.Errorf("unable to AppendListValues: Name %s, Values %s. No values to append", name, values) + } + if list, ok := gs.Status.Lists[name]; ok { + mergedList := mergeRemoveDuplicates(list.Values, values) + if len(mergedList) > int(list.Capacity) { + return errors.Errorf("unable to AppendListValues: Name %s, Values %s. Appended list length %d exceeds list capacity %d", name, values, len(mergedList), list.Capacity) + } + // If all given values are duplicates we give an error warning. + if len(mergedList) == len(list.Values) { + return errors.Errorf("unable to AppendListValues: Name %s, Values %s. All appended values are duplicates of the existing list", name, values) + } + // If only some values are duplicates, those duplicate values are silently dropped. + list.Values = mergedList + gs.Status.Lists[name] = list + return nil + } + return errors.Errorf("unable to AppendListValues: Name %s, Values %s. List not found in GameServer %s", name, values, gs.ObjectMeta.GetName()) +} + +// mergeRemoveDuplicates merges two lists and removes any duplicate values. +// Maintains ordering, so new values from list2 are appended to the end of list1. +// Returns a new list with unique values only. +func mergeRemoveDuplicates(list1 []string, list2 []string) []string { + uniqueList := []string{} + listMap := make(map[string]bool) + for _, v1 := range list1 { + if _, ok := listMap[v1]; !ok { + uniqueList = append(uniqueList, v1) + listMap[v1] = true + } + } + for _, v2 := range list2 { + if _, ok := listMap[v2]; !ok { + uniqueList = append(uniqueList, v2) + listMap[v2] = true + } + } + return uniqueList +} diff --git a/pkg/apis/agones/v1/gameserver_test.go b/pkg/apis/agones/v1/gameserver_test.go index 0123df37dd..a6cec05d15 100644 --- a/pkg/apis/agones/v1/gameserver_test.go +++ b/pkg/apis/agones/v1/gameserver_test.go @@ -1635,3 +1635,455 @@ func defaultGameServer() *GameServer { }, }, Status: GameServerStatus{State: GameServerStateCreating}} } + +func TestGameServerUpdateCount(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + gs GameServer + name string + action string + amount int64 + want CounterStatus + wantErr bool + }{ + "counter not in game server no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "foos": { + Count: 0, + Capacity: 100, + }}}}, + name: "foo", + action: "Increment", + amount: 1, + wantErr: true, + }, + "amount less than zero no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "foos": { + Count: 1, + Capacity: 100, + }}}}, + name: "foos", + action: "Decrement", + amount: -1, + want: CounterStatus{ + Count: 1, + Capacity: 100, + }, + wantErr: true, + }, + "increment by 1": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "players": { + Count: 0, + Capacity: 100, + }}}}, + name: "players", + action: "Increment", + amount: 1, + want: CounterStatus{ + Count: 1, + Capacity: 100, + }, + wantErr: false, + }, + "decrement by 10": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "bars": { + Count: 99, + Capacity: 100, + }}}}, + name: "bars", + action: "Decrement", + amount: 10, + want: CounterStatus{ + Count: 89, + Capacity: 100, + }, + wantErr: false, + }, + "incorrect action no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "bazes": { + Count: 99, + Capacity: 100, + }}}}, + name: "bazes", + action: "decrement", + amount: 10, + want: CounterStatus{ + Count: 99, + Capacity: 100, + }, + wantErr: true, + }, + "decrement beyond count no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "baz": { + Count: 99, + Capacity: 100, + }}}}, + name: "baz", + action: "Decrement", + amount: 100, + want: CounterStatus{ + Count: 99, + Capacity: 100, + }, + wantErr: true, + }, + "increment beyond capacity no-op and error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "splayers": { + Count: 99, + Capacity: 100, + }}}}, + name: "splayers", + action: "Increment", + amount: 2, + want: CounterStatus{ + Count: 99, + Capacity: 100, + }, + wantErr: true, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + err := testCase.gs.UpdateCount(testCase.name, testCase.action, testCase.amount) + if err != nil { + assert.True(t, testCase.wantErr) + } else { + assert.False(t, testCase.wantErr) + } + if counter, ok := testCase.gs.Status.Counters[testCase.name]; ok { + assert.Equal(t, testCase.want, counter) + } + }) + } +} + +func TestGameServerUpdateCounterCapacity(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + gs GameServer + name string + capacity int64 + want CounterStatus + wantErr bool + }{ + "counter not in game server no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "foos": { + Count: 0, + Capacity: 100, + }}}}, + name: "foo", + capacity: 1000, + wantErr: true, + }, + "capacity less than zero no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "foos": { + Count: 0, + Capacity: 100, + }}}}, + name: "foos", + capacity: -1000, + want: CounterStatus{ + Count: 0, + Capacity: 100, + }, + wantErr: true, + }, + "update capacity": { + gs: GameServer{Status: GameServerStatus{ + Counters: map[string]CounterStatus{ + "sessions": { + Count: 0, + Capacity: 100, + }}}}, + name: "sessions", + capacity: 9223372036854775807, + want: CounterStatus{ + Count: 0, + Capacity: 9223372036854775807, + }, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + err := testCase.gs.UpdateCounterCapacity(testCase.name, testCase.capacity) + if err != nil { + assert.True(t, testCase.wantErr) + } else { + assert.False(t, testCase.wantErr) + } + if counter, ok := testCase.gs.Status.Counters[testCase.name]; ok { + assert.Equal(t, testCase.want, counter) + } + }) + } +} + +func TestGameServerUpdateListCapacity(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + gs GameServer + name string + capacity int64 + want ListStatus + wantErr bool + }{ + "list not in game server no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "things": { + Values: []string{}, + Capacity: 100, + }}}}, + name: "thing", + capacity: 1000, + wantErr: true, + }, + "update list capacity": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "things": { + Values: []string{}, + Capacity: 100, + }}}}, + name: "things", + capacity: 1000, + want: ListStatus{ + Values: []string{}, + Capacity: 1000, + }, + wantErr: false, + }, + "list capacity above max no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "slings": { + Values: []string{}, + Capacity: 100, + }}}}, + name: "slings", + capacity: 10000, + want: ListStatus{ + Values: []string{}, + Capacity: 100, + }, + wantErr: true, + }, + "list capacity less than zero no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "flings": { + Values: []string{}, + Capacity: 999, + }}}}, + name: "flings", + capacity: -100, + want: ListStatus{ + Values: []string{}, + Capacity: 999, + }, + wantErr: true, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + err := testCase.gs.UpdateListCapacity(testCase.name, testCase.capacity) + if err != nil { + assert.True(t, testCase.wantErr) + } else { + assert.False(t, testCase.wantErr) + } + if list, ok := testCase.gs.Status.Lists[testCase.name]; ok { + assert.Equal(t, testCase.want, list) + } + }) + } +} + +func TestGameServerAppendListValues(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + gs GameServer + name string + values []string + want ListStatus + wantErr bool + }{ + "list not in game server no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "things": { + Values: []string{}, + Capacity: 100, + }}}}, + name: "thing", + values: []string{"thing1", "thing2", "thing3"}, + wantErr: true, + }, + "append values": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "things": { + Values: []string{"thing1"}, + Capacity: 100, + }}}}, + name: "things", + values: []string{"thing2", "thing3"}, + want: ListStatus{ + Values: []string{"thing1", "thing2", "thing3"}, + Capacity: 100, + }, + wantErr: false, + }, + "append values with silent drop of duplicates": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "games": { + Values: []string{"game0"}, + Capacity: 10, + }}}}, + name: "games", + values: []string{"game1", "game2", "game2", "game1"}, + want: ListStatus{ + Values: []string{"game0", "game1", "game2"}, + Capacity: 10, + }, + wantErr: false, + }, + "append values with silent drop of duplicates in original list": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "objects": { + Values: []string{"object1", "object2"}, + Capacity: 10, + }}}}, + name: "objects", + values: []string{"object2", "object1", "object3", "object3"}, + want: ListStatus{ + Values: []string{"object1", "object2", "object3"}, + Capacity: 10, + }, + wantErr: false, + }, + "append no values no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "blings": { + Values: []string{"bling1"}, + Capacity: 10, + }}}}, + name: "blings", + values: []string{}, + want: ListStatus{ + Values: []string{"bling1"}, + Capacity: 10, + }, + wantErr: true, + }, + "append only duplicates no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "slings": { + Values: []string{"slings1", "sling2"}, + Capacity: 4, + }}}}, + name: "blings", + values: []string{}, + want: ListStatus{ + Values: []string{"slings1", "sling2"}, + Capacity: 4, + }, + wantErr: true, + }, + "append too many values no-op with error": { + gs: GameServer{Status: GameServerStatus{ + Lists: map[string]ListStatus{ + "bananaslugs": { + Values: []string{"bananaslugs1", "bananaslug2", "bananaslug3"}, + Capacity: 5, + }}}}, + name: "bananaslugs", + values: []string{"bananaslug4", "bananaslug5", "bananaslug6"}, + want: ListStatus{ + Values: []string{"bananaslugs1", "bananaslug2", "bananaslug3"}, + Capacity: 5, + }, + wantErr: true, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + err := testCase.gs.AppendListValues(testCase.name, testCase.values) + if err != nil { + assert.True(t, testCase.wantErr) + } else { + assert.False(t, testCase.wantErr) + } + if list, ok := testCase.gs.Status.Lists[testCase.name]; ok { + assert.Equal(t, testCase.want, list) + } + }) + } +} + +func TestMergeRemoveDuplicates(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + str1 []string + str2 []string + want []string + }{ + "empty string arrays": { + str1: []string{}, + str2: []string{}, + want: []string{}, + }, + "no duplicates": { + str1: []string{"one"}, + str2: []string{"two", "three"}, + want: []string{"one", "two", "three"}, + }, + "remove one duplicate": { + str1: []string{"one", "one", "one"}, + str2: []string{"one", "one", "one"}, + want: []string{"one"}, + }, + "remove multiple duplicates": { + str1: []string{"one", "two"}, + str2: []string{"two", "one"}, + want: []string{"one", "two"}, + }, + } + + for test, testCase := range testCases { + t.Run(test, func(t *testing.T) { + got := mergeRemoveDuplicates(testCase.str1, testCase.str2) + assert.Equal(t, testCase.want, got) + }) + } +} diff --git a/pkg/apis/allocation/v1/gameserverallocation.go b/pkg/apis/allocation/v1/gameserverallocation.go index 7a98c36d4b..04ca4d294d 100644 --- a/pkg/apis/allocation/v1/gameserverallocation.go +++ b/pkg/apis/allocation/v1/gameserverallocation.go @@ -15,6 +15,7 @@ package v1 import ( + "errors" "fmt" "agones.dev/agones/pkg/apis" @@ -109,6 +110,12 @@ type GameServerAllocationSpec struct { // MetaPatch is optional custom metadata that is added to the game server at allocation // You can use this to tell the server necessary session data MetaPatch MetaPatch `json:"metadata,omitempty"` + + // (Alpha, CountsAndLists feature flag) Counters and Lists provide a set of actions to perform + // on Counters and Lists during allocation. + // +optional + Counters map[string]CounterAction `json:"counters,omitempty"` + Lists map[string]ListAction `json:"lists,omitempty"` } // GameServerSelector contains all the filter options for selecting @@ -166,6 +173,24 @@ type ListSelector struct { MaxAvailable int64 `json:"maxAvailable"` } +// CounterAction is an optional action that can be performed on a Counter at allocation. +// Action: "Increment" or "Decrement" the Counter's Count (optional). Must also define the Amount. +// Amount: The amount to increment or decrement the Count (optional). Must be a positive integer. +// Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max int64. +type CounterAction struct { + Action *string `json:"action,omitempty"` + Amount *int64 `json:"amount,omitempty"` + Capacity *int64 `json:"capacity,omitempty"` +} + +// ListAction is an optional action that can be performed on a List at allocation. +// AddValues: Append values to a List's Values array (optional). Any duplicate values will be ignored. +// Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000. +type ListAction struct { + AddValues []string `json:"addValues,omitempty"` + Capacity *int64 `json:"capacity,omitempty"` +} + // ApplyDefaults applies default values func (s *GameServerSelector) ApplyDefaults() { if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) { @@ -270,6 +295,44 @@ func (s *GameServerSelector) matchCounters(gs *agonesv1.GameServer) bool { return true } +// CounterActions attempts to peform any actions from the CounterAction on the GameServer Counter. +// Returns the errors of any actions that could not be performed. +func (ca *CounterAction) CounterActions(counter string, gs *agonesv1.GameServer) error { + var errs error + if ca.Capacity != nil { + capErr := gs.UpdateCounterCapacity(counter, *ca.Capacity) + if capErr != nil { + errs = errors.Join(errs, capErr) + } + } + if ca.Action != nil && ca.Amount != nil { + cntErr := gs.UpdateCount(counter, *ca.Action, *ca.Amount) + if cntErr != nil { + errs = errors.Join(errs, cntErr) + } + } + return errs +} + +// ListActions attempts to peform any actions from the ListAction on the GameServer List. +// Returns a string list of any actions that could not be performed. +func (la *ListAction) ListActions(list string, gs *agonesv1.GameServer) error { + var errs error + if la.Capacity != nil { + capErr := gs.UpdateListCapacity(list, *la.Capacity) + if capErr != nil { + errs = errors.Join(errs, capErr) + } + } + if la.AddValues != nil && len(la.AddValues) > 0 { + cntErr := gs.AppendListValues(list, la.AddValues) + if cntErr != nil { + errs = errors.Join(errs, cntErr) + } + } + return errs +} + // matchLists returns true if there is a match for the ListSelector in the GameServerStatus func (s *GameServerSelector) matchLists(gs *agonesv1.GameServer) bool { if gs.Status.Lists == nil { diff --git a/pkg/apis/allocation/v1/gameserverallocation_test.go b/pkg/apis/allocation/v1/gameserverallocation_test.go index b412efa7f1..fdf754917f 100644 --- a/pkg/apis/allocation/v1/gameserverallocation_test.go +++ b/pkg/apis/allocation/v1/gameserverallocation_test.go @@ -1016,6 +1016,231 @@ func TestGameServerSelectorMatches(t *testing.T) { } } +// Helper function for creating int64 pointers +func int64Pointer(x int64) *int64 { + return &x +} + +func TestGameServerCounterActions(t *testing.T) { + t.Parallel() + + runtime.FeatureTestMutex.Lock() + defer runtime.FeatureTestMutex.Unlock() + assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists))) + + DECREMENT := "Decrement" + INCREMENT := "Increment" + + testScenarios := map[string]struct { + ca CounterAction + counter string + gs *agonesv1.GameServer + want *agonesv1.GameServer + wantErr bool + }{ + "update counter capacity": { + ca: CounterAction{ + Capacity: int64Pointer(0), + }, + counter: "mages", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "mages": { + Count: 1, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "mages": { + Count: 1, + Capacity: 0, + }}}}, + wantErr: false, + }, + "fail update counter capacity and count": { + ca: CounterAction{ + Action: &INCREMENT, + Amount: int64Pointer(10), + Capacity: int64Pointer(-1), + }, + counter: "sages", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "sages": { + Count: 99, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "sages": { + Count: 99, + Capacity: 100, + }}}}, + wantErr: true, + }, + "update counter count": { + ca: CounterAction{ + Action: &INCREMENT, + Amount: int64Pointer(10), + }, + counter: "baddies", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "baddies": { + Count: 1, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "baddies": { + Count: 11, + Capacity: 100, + }}}}, + wantErr: false, + }, + "update counter count and capacity": { + ca: CounterAction{ + Action: &DECREMENT, + Amount: int64Pointer(10), + Capacity: int64Pointer(10), + }, + counter: "heroes", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "heroes": { + Count: 11, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Counters: map[string]agonesv1.CounterStatus{ + "heroes": { + Count: 1, + Capacity: 10, + }}}}, + wantErr: false, + }, + } + + for test, testScenario := range testScenarios { + t.Run(test, func(t *testing.T) { + errs := testScenario.ca.CounterActions(testScenario.counter, testScenario.gs) + if errs != nil { + assert.True(t, testScenario.wantErr) + } else { + assert.False(t, testScenario.wantErr) + } + assert.Equal(t, testScenario.want, testScenario.gs) + }) + } +} + +func TestGameServerListActions(t *testing.T) { + t.Parallel() + + runtime.FeatureTestMutex.Lock() + defer runtime.FeatureTestMutex.Unlock() + assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists))) + + testScenarios := map[string]struct { + la ListAction + list string + gs *agonesv1.GameServer + want *agonesv1.GameServer + wantErr bool + }{ + "update list capacity": { + la: ListAction{ + Capacity: int64Pointer(0), + }, + list: "pages", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "pages": { + Values: []string{"page1", "page2"}, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "pages": { + Values: []string{"page1", "page2"}, + Capacity: 0, + }}}}, + wantErr: false, + }, + "update list values": { + la: ListAction{ + AddValues: []string{"sage1", "sage3"}, + }, + list: "sages", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "sages": { + Values: []string{"sage1", "sage2"}, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "sages": { + Values: []string{"sage1", "sage2", "sage3"}, + Capacity: 100, + }}}}, + wantErr: false, + }, + "update list values and capacity": { + la: ListAction{ + AddValues: []string{"magician1", "magician3"}, + Capacity: int64Pointer(42), + }, + list: "magicians", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "magicians": { + Values: []string{"magician1", "magician2"}, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "magicians": { + Values: []string{"magician1", "magician2", "magician3"}, + Capacity: 42, + }}}}, + wantErr: false, + }, + "update list values and capacity - value add fails": { + la: ListAction{ + AddValues: []string{"fairy1", "fairy3"}, + Capacity: int64Pointer(2), + }, + list: "fairies", + gs: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "fairies": { + Values: []string{"fairy1", "fairy2"}, + Capacity: 100, + }}}}, + want: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{ + Lists: map[string]agonesv1.ListStatus{ + "fairies": { + Values: []string{"fairy1", "fairy2"}, + Capacity: 2, + }}}}, + wantErr: true, + }, + } + + for test, testScenario := range testScenarios { + t.Run(test, func(t *testing.T) { + errs := testScenario.la.ListActions(testScenario.list, testScenario.gs) + if errs != nil { + assert.True(t, testScenario.wantErr) + } else { + assert.False(t, testScenario.wantErr) + } + assert.Equal(t, testScenario.want, testScenario.gs) + }) + } +} + func TestGameServerAllocationValidate(t *testing.T) { t.Parallel() diff --git a/pkg/apis/allocation/v1/zz_generated.deepcopy.go b/pkg/apis/allocation/v1/zz_generated.deepcopy.go index e532bb88a6..f08064e317 100644 --- a/pkg/apis/allocation/v1/zz_generated.deepcopy.go +++ b/pkg/apis/allocation/v1/zz_generated.deepcopy.go @@ -26,6 +26,37 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CounterAction) DeepCopyInto(out *CounterAction) { + *out = *in + if in.Action != nil { + in, out := &in.Action, &out.Action + *out = new(string) + **out = **in + } + if in.Amount != nil { + in, out := &in.Amount, &out.Amount + *out = new(int64) + **out = **in + } + if in.Capacity != nil { + in, out := &in.Capacity, &out.Capacity + *out = new(int64) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CounterAction. +func (in *CounterAction) DeepCopy() *CounterAction { + if in == nil { + return nil + } + out := new(CounterAction) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CounterSelector) DeepCopyInto(out *CounterSelector) { *out = *in @@ -128,6 +159,20 @@ func (in *GameServerAllocationSpec) DeepCopyInto(out *GameServerAllocationSpec) } } in.MetaPatch.DeepCopyInto(&out.MetaPatch) + if in.Counters != nil { + in, out := &in.Counters, &out.Counters + *out = make(map[string]CounterAction, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } + if in.Lists != nil { + in, out := &in.Lists, &out.Lists + *out = make(map[string]ListAction, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } return } @@ -203,6 +248,32 @@ func (in *GameServerSelector) DeepCopy() *GameServerSelector { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ListAction) DeepCopyInto(out *ListAction) { + *out = *in + if in.AddValues != nil { + in, out := &in.AddValues, &out.AddValues + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Capacity != nil { + in, out := &in.Capacity, &out.Capacity + *out = new(int64) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ListAction. +func (in *ListAction) DeepCopy() *ListAction { + if in == nil { + return nil + } + out := new(ListAction) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ListSelector) DeepCopyInto(out *ListSelector) { *out = *in diff --git a/pkg/gameserverallocations/allocator.go b/pkg/gameserverallocations/allocator.go index 1b7ad87521..58b8703a35 100644 --- a/pkg/gameserverallocations/allocator.go +++ b/pkg/gameserverallocations/allocator.go @@ -18,6 +18,7 @@ import ( "context" "crypto/tls" "crypto/x509" + goErrors "errors" "fmt" "net/http" "strings" @@ -553,7 +554,7 @@ func (c *Allocator) allocationUpdateWorkers(ctx context.Context, workerCount int for { select { case res := <-updateQueue: - gs, err := c.applyAllocationToGameServer(ctx, res.request.gsa.Spec.MetaPatch, res.gs) + gs, err := c.applyAllocationToGameServer(ctx, res.request.gsa.Spec.MetaPatch, res.gs, res.request.gsa) if err != nil { if !k8serrors.IsConflict(errors.Cause(err)) { // since we could not allocate, we should put it back @@ -564,7 +565,6 @@ func (c *Allocator) allocationUpdateWorkers(ctx context.Context, workerCount int res.err = errors.Wrap(err, "error updating allocated gameserver") } else { res.gs = gs - c.recorder.Event(res.gs, corev1.EventTypeNormal, string(res.gs.Status.State), "Allocated") } res.request.response <- res @@ -580,7 +580,7 @@ func (c *Allocator) allocationUpdateWorkers(ctx context.Context, workerCount int // applyAllocationToGameServer patches the inputted GameServer with the allocation metadata changes, and updates it to the Allocated State. // Returns the updated GameServer. -func (c *Allocator) applyAllocationToGameServer(ctx context.Context, mp allocationv1.MetaPatch, gs *agonesv1.GameServer) (*agonesv1.GameServer, error) { +func (c *Allocator) applyAllocationToGameServer(ctx context.Context, mp allocationv1.MetaPatch, gs *agonesv1.GameServer, gsa *allocationv1.GameServerAllocation) (*agonesv1.GameServer, error) { // patch ObjectMeta labels if mp.Labels != nil { if gs.ObjectMeta.Labels == nil { @@ -607,7 +607,37 @@ func (c *Allocator) applyAllocationToGameServer(ctx context.Context, mp allocati gs.ObjectMeta.Annotations[LastAllocatedAnnotationKey] = string(ts) gs.Status.State = agonesv1.GameServerStateAllocated - return c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gs, metav1.UpdateOptions{}) + // perfom any Counter or List actions + var counterErrors error + var listErrors error + if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + if gsa.Spec.Counters != nil { + for counter, ca := range gsa.Spec.Counters { + counterErrors = goErrors.Join(counterErrors, ca.CounterActions(counter, gs)) + } + } + if gsa.Spec.Lists != nil { + for list, la := range gsa.Spec.Lists { + listErrors = goErrors.Join(listErrors, la.ListActions(list, gs)) + } + } + } + + gsUpdate, updateErr := c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gs, metav1.UpdateOptions{}) + if updateErr != nil { + return gsUpdate, updateErr + } + + // If successful Update record any Counter or List action errors as a warning + if counterErrors != nil { + c.recorder.Event(gsUpdate, corev1.EventTypeWarning, "CounterActionError", counterErrors.Error()) + } + if listErrors != nil { + c.recorder.Event(gsUpdate, corev1.EventTypeWarning, "ListActionError", listErrors.Error()) + } + c.recorder.Event(gsUpdate, corev1.EventTypeNormal, string(gsUpdate.Status.State), "Allocated") + + return gsUpdate, updateErr } // Retry retries fn based on backoff provided. diff --git a/pkg/gameserverallocations/allocator_test.go b/pkg/gameserverallocations/allocator_test.go index 0f4a9ae3f8..48e17b7cf5 100644 --- a/pkg/gameserverallocations/allocator_test.go +++ b/pkg/gameserverallocations/allocator_test.go @@ -17,6 +17,7 @@ package gameserverallocations import ( "context" "errors" + "fmt" "testing" "time" @@ -26,6 +27,7 @@ import ( multiclusterv1 "agones.dev/agones/pkg/apis/multicluster/v1" "agones.dev/agones/pkg/gameservers" agtesting "agones.dev/agones/pkg/testing" + "agones.dev/agones/pkg/util/runtime" "github.com/heptiolabs/healthcheck" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -245,14 +247,14 @@ func TestAllocatorApplyAllocationToGameServer(t *testing.T) { time.Second, 5*time.Second, 500*time.Millisecond, ) - gs, err := allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{}, &agonesv1.GameServer{}) + gs, err := allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{}, &agonesv1.GameServer{}, &allocationv1.GameServerAllocation{}) assert.NoError(t, err) assert.Equal(t, agonesv1.GameServerStateAllocated, gs.Status.State) assert.NotNil(t, gs.ObjectMeta.Annotations["agones.dev/last-allocated"]) var ts time.Time assert.NoError(t, ts.UnmarshalText([]byte(gs.ObjectMeta.Annotations[LastAllocatedAnnotationKey]))) - gs, err = allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{Labels: map[string]string{"foo": "bar"}}, &agonesv1.GameServer{}) + gs, err = allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{Labels: map[string]string{"foo": "bar"}}, &agonesv1.GameServer{}, &allocationv1.GameServerAllocation{}) assert.NoError(t, err) assert.Equal(t, agonesv1.GameServerStateAllocated, gs.Status.State) assert.Equal(t, "bar", gs.ObjectMeta.Labels["foo"]) @@ -260,7 +262,7 @@ func TestAllocatorApplyAllocationToGameServer(t *testing.T) { gs, err = allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{Labels: map[string]string{"foo": "bar"}, Annotations: map[string]string{"bar": "foo"}}, - &agonesv1.GameServer{}) + &agonesv1.GameServer{}, &allocationv1.GameServerAllocation{}) assert.NoError(t, err) assert.Equal(t, agonesv1.GameServerStateAllocated, gs.Status.State) assert.Equal(t, "bar", gs.ObjectMeta.Labels["foo"]) @@ -268,6 +270,152 @@ func TestAllocatorApplyAllocationToGameServer(t *testing.T) { assert.NotNil(t, gs.ObjectMeta.Annotations[LastAllocatedAnnotationKey]) } +func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) { + t.Parallel() + m := agtesting.NewMocks() + ctx := context.Background() + mp := allocationv1.MetaPatch{} + + m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, k8sruntime.Object, error) { + ua := action.(k8stesting.UpdateAction) + gs := ua.GetObject().(*agonesv1.GameServer) + return true, gs, nil + }) + + allocator := NewAllocator(m.AgonesInformerFactory.Multicluster().V1().GameServerAllocationPolicies(), + m.KubeInformerFactory.Core().V1().Secrets(), + m.AgonesClient.AgonesV1(), m.KubeClient, + NewAllocationCache(m.AgonesInformerFactory.Agones().V1().GameServers(), gameservers.NewPerNodeCounter(m.KubeInformerFactory, m.AgonesInformerFactory), healthcheck.NewHandler()), + time.Second, 5*time.Second, 500*time.Millisecond, + ) + + ONE := int64(1) + FORTY := int64(40) + THOUSAND := int64(1000) + INCREMENT := "Increment" + READY := agonesv1.GameServerStateReady + + gs1 := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "gs1", Namespace: defaultNs, UID: "1"}, + Status: agonesv1.GameServerStatus{NodeName: "node1", State: agonesv1.GameServerStateReady, + Lists: map[string]agonesv1.ListStatus{ + "players": { + Values: []string{}, + Capacity: 100, + }, + }, + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 101, + Capacity: 1000, + }}}} + gs2 := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "gs2", Namespace: defaultNs, UID: "2"}, + Status: agonesv1.GameServerStatus{NodeName: "node1", State: agonesv1.GameServerStateReady, + Lists: map[string]agonesv1.ListStatus{ + "players": { + Values: []string{}, + Capacity: 100, + }, + }, + Counters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 101, + Capacity: 1000, + }}}} + + testScenarios := map[string]struct { + features string + gs *agonesv1.GameServer + gsa *allocationv1.GameServerAllocation + wantCounters map[string]agonesv1.CounterStatus + wantLists map[string]agonesv1.ListStatus + }{ + "CounterActions increment and ListActions append and update capacity": { + features: fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists), + gs: &gs1, + gsa: &allocationv1.GameServerAllocation{ + ObjectMeta: metav1.ObjectMeta{Namespace: defaultNs}, + Spec: allocationv1.GameServerAllocationSpec{ + Selectors: []allocationv1.GameServerSelector{{ + GameServerState: &READY, + }}, + Scheduling: apis.Packed, + Counters: map[string]allocationv1.CounterAction{ + "rooms": { + Action: &INCREMENT, + Amount: &ONE, + }}, + Lists: map[string]allocationv1.ListAction{ + "players": { + AddValues: []string{"x7un", "8inz"}, + Capacity: &FORTY, + }}}}, + wantCounters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 102, + Capacity: 1000, + }}, + wantLists: map[string]agonesv1.ListStatus{ + "players": { + Values: []string{"x7un", "8inz"}, + Capacity: 40, + }}, + }, + "CounterActions and ListActions only update list capacity": { + features: fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists), + gs: &gs2, + gsa: &allocationv1.GameServerAllocation{ + ObjectMeta: metav1.ObjectMeta{Namespace: defaultNs}, + Spec: allocationv1.GameServerAllocationSpec{ + Selectors: []allocationv1.GameServerSelector{{ + GameServerState: &READY, + }}, + Scheduling: apis.Packed, + Counters: map[string]allocationv1.CounterAction{ + "rooms": { + Action: &INCREMENT, + Amount: &THOUSAND, + }}, + Lists: map[string]allocationv1.ListAction{ + "players": { + AddValues: []string{"x7un", "8inz"}, + Capacity: &ONE, + }}}}, + wantCounters: map[string]agonesv1.CounterStatus{ + "rooms": { + Count: 101, + Capacity: 1000, + }}, + wantLists: map[string]agonesv1.ListStatus{ + "players": { + Values: []string{}, + Capacity: 1, + }}, + }, + } + + for test, testScenario := range testScenarios { + t.Run(test, func(t *testing.T) { + runtime.FeatureTestMutex.Lock() + defer runtime.FeatureTestMutex.Unlock() + // we always set the feature flag in all these tests, so always process it. + require.NoError(t, runtime.ParseFeatures(testScenario.features)) + + foundGs, err := allocator.applyAllocationToGameServer(ctx, mp, testScenario.gs, testScenario.gsa) + assert.NoError(t, err) + for counter, counterStatus := range testScenario.wantCounters { + if gsCounter, ok := foundGs.Status.Counters[counter]; ok { + assert.Equal(t, counterStatus, gsCounter) + } + } + for list, listStatus := range testScenario.wantLists { + if gsList, ok := foundGs.Status.Lists[list]; ok { + assert.Equal(t, listStatus, gsList) + } + } + }) + } +} + func TestAllocationApplyAllocationError(t *testing.T) { t.Parallel() m := agtesting.NewMocks() @@ -284,7 +432,7 @@ func TestAllocationApplyAllocationError(t *testing.T) { time.Second, 5*time.Second, 500*time.Millisecond, ) - gsa, err := allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{}, &agonesv1.GameServer{}) + gsa, err := allocator.applyAllocationToGameServer(ctx, allocationv1.MetaPatch{}, &agonesv1.GameServer{}, &allocationv1.GameServerAllocation{}) logrus.WithError(err).WithField("gsa", gsa).WithField("test", t.Name()).Info("Allocation should fail") assert.Error(t, err) } diff --git a/site/content/en/docs/Reference/agones_crd_api_reference.html b/site/content/en/docs/Reference/agones_crd_api_reference.html index 45c685a2f2..df62bde126 100644 --- a/site/content/en/docs/Reference/agones_crd_api_reference.html +++ b/site/content/en/docs/Reference/agones_crd_api_reference.html @@ -5704,6 +5704,33 @@

GameServerAllocation You can use this to tell the server necessary session data

+ + +counters
+ + +map[string]agones.dev/agones/pkg/apis/allocation/v1.CounterAction + + + + +(Optional) +

(Alpha, CountsAndLists feature flag) Counters and Lists provide a set of actions to perform +on Counters and Lists during allocation.

+ + + + +lists
+ + +map[string]agones.dev/agones/pkg/apis/allocation/v1.ListAction + + + + + + @@ -5721,6 +5748,58 @@

GameServerAllocation +

CounterAction +

+

+(Appears on: +GameServerAllocationSpec) +

+

+

CounterAction is an optional action that can be performed on a Counter at allocation. +Action: “Increment” or “Decrement” the Counter’s Count (optional). Must also define the Amount. +Amount: The amount to increment or decrement the Count (optional). Must be a positive integer. +Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max int64.

+

+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+action
+ +string + +		 +
+amount
+ +int64 + +		 +
+capacity
+ +int64 + +		 +

CounterSelector

@@ -5903,6 +5982,33 @@

GameServerAllocationS You can use this to tell the server necessary session data

+ + +counters
+ + +map[string]agones.dev/agones/pkg/apis/allocation/v1.CounterAction + + + + +(Optional) +

(Alpha, CountsAndLists feature flag) Counters and Lists provide a set of actions to perform +on Counters and Lists during allocation.

+ + + + +lists
+ + +map[string]agones.dev/agones/pkg/apis/allocation/v1.ListAction + + + + + +

GameServerAllocationState @@ -6103,6 +6209,47 @@

GameServerSelector +

ListAction +

+

+(Appears on: +GameServerAllocationSpec) +

+

+

ListAction is an optional action that can be performed on a List at allocation. +AddValues: Append values to a List’s Values array (optional). Any duplicate values will be ignored. +Capacity: Update the maximum capacity of the Counter to this number (optional). Min 0, Max 1000.

+

+ + + + + + + + + + + + + + + + + +
FieldDescription
+addValues
+ +[]string + +		 +
+capacity
+ +int64 + +		 +

ListSelector