Skip to content

Commit

Permalink
GameServerAllocation to sort Priorities by Allocated Capacity (google…
Browse files Browse the repository at this point in the history 
…forgames#3282)

* Updates GameServerAllocation to sort by Allocated Capacity when sorting by Priorities.

Removes a duplicate import of k8s.io/apimachinery/pkg/api/validation.
Adds in a SortKey for GameServerAllocation Priorities  to differentiate between requests with different priorities and / or scheduling during  ListenAndAllocate.

* New hashstructure vendor and generate files

* Minor changing variable from pointer to uint64
  • Loading branch information
@igooch
igooch authored Jul 26, 2023
1 parent 486860c commit 53d30db
Show file tree
Hide file tree
Showing 15 changed files with 2,719 additions and 1,827 deletions.
1 change: 1 addition & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ require (
github.com/joonix/log v0.0.0-20180502111528-d2d3f2f4a806
github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a
github.com/mennanov/fmutils v0.2.0
github.com/mitchellh/hashstructure/v2 v2.0.2
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.14.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,8 @@ github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceT
github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eIhFbs2+BFgRtFPeD/KE+zxI=
github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg=
github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4=
github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE=
github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY=
github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
Expand Down
48 changes: 30 additions & 18 deletions pkg/apis/allocation/v1/gameserverallocation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"agones.dev/agones/pkg/apis"
agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
"agones.dev/agones/pkg/util/runtime"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
hashstructure "github.com/mitchellh/hashstructure/v2"
apivalidation "k8s.io/apimachinery/pkg/api/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
Expand Down Expand Up @@ -69,46 +69,46 @@ type GameServerAllocationList struct {
type GameServerAllocationSpec struct {
// MultiClusterPolicySelector if specified, multi-cluster policies are applied.
// Otherwise, allocation will happen locally.
MultiClusterSetting MultiClusterSetting `json:"multiClusterSetting,omitempty"`
MultiClusterSetting MultiClusterSetting `json:"multiClusterSetting,omitempty" hash:"ignore"`

// Deprecated: use field Selectors instead. If Selectors is set, this field is ignored.
// Required is the GameServer selector from which to choose GameServers from.
// Defaults to all GameServers.
Required GameServerSelector `json:"required,omitempty"`
Required GameServerSelector `json:"required,omitempty" hash:"ignore"`

// Deprecated: use field Selectors instead. If Selectors is set, this field is ignored.
// Preferred is an ordered list of preferred GameServer selectors
// that are optional to be fulfilled, but will be searched before the `required` selector.
// If the first selector is not matched, the selection attempts the second selector, and so on.
// If any of the preferred selectors are matched, the required selector is not considered.
// This is useful for things like smoke testing of new game servers.
Preferred []GameServerSelector `json:"preferred,omitempty"`
Preferred []GameServerSelector `json:"preferred,omitempty" hash:"ignore"`

// (Alpha, CountsAndLists feature flag) The first Priority on the array of Priorities is the most
// important for sorting. The allocator will use the first priority for sorting GameServers by
// available Capacity in the Selector set and acts as a tie-breaker after sorting the game servers
// by State and Strategy. Impacts which GameServer is checked first.
// available Capacity in the Selector set. Acts as a tie-breaker after sorting the game servers
// by State and Strategy Packed. Impacts which GameServer is checked first.
// +optional
Priorities []agonesv1.Priority `json:"priorities,omitempty"`

// Ordered list of GameServer label selectors.
// If the first selector is not matched, the selection attempts the second selector, and so on.
// This is useful for things like smoke testing of new game servers.
// Note: This field can only be set if neither Required or Preferred is set.
Selectors []GameServerSelector `json:"selectors,omitempty"`
Selectors []GameServerSelector `json:"selectors,omitempty" hash:"ignore"`

// Scheduling strategy. Defaults to "Packed".
Scheduling apis.SchedulingStrategy `json:"scheduling"`

// 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"`
MetaPatch MetaPatch `json:"metadata,omitempty" hash:"ignore"`

// (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"`
Counters map[string]CounterAction `json:"counters,omitempty" hash:"ignore"`
Lists map[string]ListAction `json:"lists,omitempty" hash:"ignore"`
}

// GameServerSelector contains all the filter options for selecting
Expand Down Expand Up @@ -378,11 +378,11 @@ func (s *GameServerSelector) Validate(fldPath *field.Path) field.ErrorList {

if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) && s.Players != nil {
if s.Players.MinAvailable < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("players").Child("minAvailable"), s.Players.MinAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(fldPath.Child("players").Child("minAvailable"), s.Players.MinAvailable, apivalidation.IsNegativeErrorMsg))
}

if s.Players.MaxAvailable < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("players").Child("maxAvailable"), s.Players.MaxAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(fldPath.Child("players").Child("maxAvailable"), s.Players.MaxAvailable, apivalidation.IsNegativeErrorMsg))
}

if s.Players.MinAvailable > s.Players.MaxAvailable {
Expand Down Expand Up @@ -415,19 +415,19 @@ func validateCounters(counters map[string]CounterSelector, fldPath *field.Path)
for key, counterSelector := range counters {
keyPath := fldPath.Key(key)
if counterSelector.MinCount < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("minCount"), counterSelector.MinCount, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("minCount"), counterSelector.MinCount, apivalidation.IsNegativeErrorMsg))
}
if counterSelector.MaxCount < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxCount"), counterSelector.MaxCount, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxCount"), counterSelector.MaxCount, apivalidation.IsNegativeErrorMsg))
}
if (counterSelector.MaxCount < counterSelector.MinCount) && (counterSelector.MaxCount != 0) {
allErrs = append(allErrs, field.Invalid(keyPath, counterSelector.MaxCount, fmt.Sprintf("maxCount must zero or greater than minCount %d", counterSelector.MinCount)))
}
if counterSelector.MinAvailable < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("minAvailable"), counterSelector.MinAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("minAvailable"), counterSelector.MinAvailable, apivalidation.IsNegativeErrorMsg))
}
if counterSelector.MaxAvailable < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxAvailable"), counterSelector.MaxAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxAvailable"), counterSelector.MaxAvailable, apivalidation.IsNegativeErrorMsg))
}
if (counterSelector.MaxAvailable < counterSelector.MinAvailable) && (counterSelector.MaxAvailable != 0) {
allErrs = append(allErrs, field.Invalid(keyPath, counterSelector.MaxAvailable, fmt.Sprintf("maxAvailable must zero or greater than minAvailable %d", counterSelector.MinAvailable)))
Expand All @@ -443,10 +443,10 @@ func validateLists(lists map[string]ListSelector, fldPath *field.Path) field.Err
for key, listSelector := range lists {
keyPath := fldPath.Key(key)
if listSelector.MinAvailable < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("minAvailable"), listSelector.MinAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("minAvailable"), listSelector.MinAvailable, apivalidation.IsNegativeErrorMsg))
}
if listSelector.MaxAvailable < 0 {
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxAvailable"), listSelector.MaxAvailable, apimachineryvalidation.IsNegativeErrorMsg))
allErrs = append(allErrs, field.Invalid(keyPath.Child("maxAvailable"), listSelector.MaxAvailable, apivalidation.IsNegativeErrorMsg))
}
if (listSelector.MaxAvailable < listSelector.MinAvailable) && (listSelector.MaxAvailable != 0) {
allErrs = append(allErrs, field.Invalid(keyPath, listSelector.MaxAvailable, fmt.Sprintf("maxAvailable must zero or greater than minAvailable %d", listSelector.MinAvailable)))
Expand Down Expand Up @@ -549,3 +549,15 @@ func (gsa *GameServerAllocation) Converter() {
gsa.Spec.Selectors = selectors
}
}

// SortKey generates and returns the hash of the GameServerAllocationSpec []Priority and Scheduling.
// Note: The hash:"ignore" in GameServerAllocationSpec means that these fields will not be considered
// in hashing. The hash is used for determining when GameServerAllocations have equal or different
// []Priority and Scheduling.
func (gsa *GameServerAllocation) SortKey() (uint64, error) {
hash, err := hashstructure.Hash(gsa.Spec, hashstructure.FormatV2, nil)
if err != nil {
return 0, err
}
return hash, nil
}
114 changes: 114 additions & 0 deletions pkg/apis/allocation/v1/gameserverallocation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1132,3 +1132,117 @@ func TestGameServerAllocationConverter(t *testing.T) {
gsa.Converter()
assert.Equal(t, gsaExpected, gsa)
}

func TestSortKey(t *testing.T) {
t.Parallel()

runtime.FeatureTestMutex.Lock()
defer runtime.FeatureTestMutex.Unlock()
assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists)))

gameServerAllocation1 := &GameServerAllocation{
Spec: GameServerAllocationSpec{
Scheduling: "Packed",
Priorities: []agonesv1.Priority{
{
Type: "List",
Key: "foo",
Order: "Descending",
},
},
},
}

gameServerAllocation2 := &GameServerAllocation{
Spec: GameServerAllocationSpec{
Selectors: []GameServerSelector{
{LabelSelector: metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}},
},
Scheduling: "Packed",
Priorities: []agonesv1.Priority{
{
Type: "List",
Key: "foo",
Order: "Descending",
},
},
},
}

gameServerAllocation3 := &GameServerAllocation{
Spec: GameServerAllocationSpec{
Scheduling: "Packed",
Priorities: []agonesv1.Priority{
{
Type: "Counter",
Key: "foo",
Order: "Descending",
},
},
},
}

gameServerAllocation4 := &GameServerAllocation{
Spec: GameServerAllocationSpec{
Scheduling: "Distributed",
Priorities: []agonesv1.Priority{
{
Type: "List",
Key: "foo",
Order: "Descending",
},
},
},
}

gameServerAllocation5 := &GameServerAllocation{}

gameServerAllocation6 := &GameServerAllocation{
Spec: GameServerAllocationSpec{
Priorities: []agonesv1.Priority{},
},
}

testScenarios := map[string]struct {
gsa1 *GameServerAllocation
gsa2 *GameServerAllocation
wantEqual bool
}{
"equivalent GameServerAllocation": {
gsa1: gameServerAllocation1,
gsa2: gameServerAllocation2,
wantEqual: true,
},
"different Scheduling GameServerAllocation": {
gsa1: gameServerAllocation1,
gsa2: gameServerAllocation4,
wantEqual: false,
},
"equivalent empty GameServerAllocation": {
gsa1: gameServerAllocation5,
gsa2: gameServerAllocation6,
wantEqual: true,
},
"different Priorities GameServerAllocation": {
gsa1: gameServerAllocation1,
gsa2: gameServerAllocation3,
wantEqual: false,
},
}

for test, testScenario := range testScenarios {
t.Run(test, func(t *testing.T) {
key1, err := testScenario.gsa1.SortKey()
assert.NoError(t, err)
key2, err := testScenario.gsa2.SortKey()
assert.NoError(t, err)

if testScenario.wantEqual {
assert.Equal(t, key1, key2)
} else {
assert.NotEqual(t, key1, key2)
}
})
}

}
23 changes: 13 additions & 10 deletions pkg/gameserverallocations/allocation_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ func (c *AllocationCache) ListSortedGameServers(gsa *allocationv1.GameServerAllo
return list
}

// compareGameServers compares two game servers based on a CountsAndLists Priority
// compareGameServers compares two game servers based on a CountsAndLists Priority using available
// capacity (Capacity - Count for Counters, and Capacity - len(Values) for Lists) as the comparison.
// Returns -1 if gs1 < gs2; 1 if gs1 > gs2; 0 if gs1 == gs2; 0 if neither gamer server has the Priority.
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Priority
// Order is Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
Expand All @@ -275,13 +276,15 @@ func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int
counter2, ok2 := gs2.Status.Counters[p.Key]
// If both game servers have the Counter
if ok1 && ok2 {
if counter1.Count < counter2.Count {
availCapacity1 := counter1.Capacity - counter1.Count
availCapacity2 := counter2.Capacity - counter2.Count
if availCapacity1 < availCapacity2 {
return -1
}
if counter1.Count > counter2.Count {
if availCapacity1 > availCapacity2 {
return 1
}
if counter1.Count == counter2.Count {
if availCapacity1 == availCapacity2 {
return 0
}
}
Expand All @@ -293,13 +296,15 @@ func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int
list2, ok2 := gs2.Status.Lists[p.Key]
// If both game servers have the List
if ok1 && ok2 {
if len(list1.Values) < len(list2.Values) {
availCapacity1 := list1.Capacity - int64(len(list1.Values))
availCapacity2 := list2.Capacity - int64(len(list2.Values))
if availCapacity1 < availCapacity2 {
return -1
}
if len(list1.Values) > len(list2.Values) {
if availCapacity1 > availCapacity2 {
return 1
}
if len(list1.Values) == len(list2.Values) {
if availCapacity1 == availCapacity2 {
return 0
}
}
Expand All @@ -309,13 +314,11 @@ func compareGameServers(p *agonesv1.Priority, gs1, gs2 *agonesv1.GameServer) int
// If only one game server has the Priority, prefer that server. I.e. nil < gsX when Order is
// Descending (3, 2, 1, 0, nil), and nil > gsX when Order is Ascending (0, 1, 2, 3, nil).
if (gs1ok && p.Order == agonesv1.GameServerPriorityDescending) ||
(gs1ok && p.Order == "") ||
(gs2ok && p.Order == agonesv1.GameServerPriorityAscending) {
return 1
}
if (gs1ok && p.Order == agonesv1.GameServerPriorityAscending) ||
(gs2ok && p.Order == agonesv1.GameServerPriorityDescending) ||
(gs2ok && p.Order == "") {
(gs2ok && p.Order == agonesv1.GameServerPriorityDescending) {
return -1
}
// If neither game server has the Priority
Expand Down
Loading

0 comments on commit 53d30db

Please sign in to comment.