Skip to content

Commit

Permalink
Updates GameServerSet to only aggregate Counters and Lists on GameSer…
Browse files Browse the repository at this point in the history
…ver States Allocated, Ready, or Reserved
  • Loading branch information
igooch committed May 26, 2023
1 parent f784c70 commit 84c2677
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 20 deletions.
3 changes: 0 additions & 3 deletions pkg/fleets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,6 @@ func mergeLists(l1, l2 map[string]agonesv1.AggregatedListStatus) map[string]agon

for key, val := range l2 {
// If the List exists in both maps, aggregate the values.
// TODO: Will this cause issues with install/helm/agones/templates/crds/fleet.yaml because in
// the CRD we define a maximum capacity and maximum number of values as 1000? (Also a possible
// issue with there being a max of 1000 Lists in a Fleet per the CRD?) So far it passes unit tests...
if list, ok := l1[key]; ok {
list.Capacity += val.Capacity
// We do not remove duplicates here.
Expand Down
12 changes: 5 additions & 7 deletions pkg/gameserversets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,10 +620,11 @@ func computeStatus(list []*agonesv1.GameServer) agonesv1.GameServerSetStatus {
status.ReservedReplicas++
}

// TODO: Aggregates all Counters and Lists for all GameServer States that are not Deleting. This
// means that unlike player tracking below, this will aggregate values during GameServerStateCreating.
// Should I mimic player tracking below and only aggregate for cases Ready, Reserved, and Allocated?
if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
// Aggregates all Counters and Lists only for GameServer states Ready, Reserved, or Allocated.
if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) &&
(gs.Status.State == agonesv1.GameServerStateReady ||
gs.Status.State == agonesv1.GameServerStateAllocated ||
gs.Status.State == agonesv1.GameServerStateReserved) {
status.Counters = aggregateCounters(status.Counters, gs.Status.Counters)
status.Lists = aggregateLists(status.Lists, gs.Status.Lists)
}
Expand Down Expand Up @@ -678,9 +679,6 @@ func aggregateLists(aggListStatus map[string]agonesv1.AggregatedListStatus, list

for key, val := range listStatus {
// If the List exists in both maps, aggregate the values.
// TODO: Will this cause issues with install/helm/agones/templates/crds/gameserverset.yaml because in
// the CRD we define a maximum capacity and maximum number of values as 1000? (Also a possible
// issue with there being a max of 1000 Lists in a GameServerSet per the CRD?)
if list, ok := aggListStatus[key]; ok {
list.Capacity += val.Capacity
// We do not remove duplicates here.
Expand Down
22 changes: 12 additions & 10 deletions pkg/gameserversets/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,8 @@ func TestComputeStatus(t *testing.T) {
}
gs4 := gsWithState(agonesv1.GameServerStateReady)
gs4.Status.Counters = map[string]agonesv1.CounterStatus{
"firstCounter": {Count: 15, Capacity: 30},
"firstCounter": {Count: 15, Capacity: 30},
"secondCounter": {Count: 20, Capacity: 200},
}
list = append(list, gs1, gs2, gs3, gs4)

Expand All @@ -370,12 +371,12 @@ func TestComputeStatus(t *testing.T) {
AllocatedReplicas: 1,
Counters: map[string]agonesv1.AggregatedCounterStatus{
"firstCounter": {
Count: 50,
Capacity: 85,
Count: 30,
Capacity: 55,
},
"secondCounter": {
Count: 200,
Capacity: 2000,
Count: 120,
Capacity: 1200,
},
},
Lists: map[string]agonesv1.AggregatedListStatus{},
Expand Down Expand Up @@ -407,7 +408,8 @@ func TestComputeStatus(t *testing.T) {
}
gs4 := gsWithState(agonesv1.GameServerStateReady)
gs4.Status.Lists = map[string]agonesv1.ListStatus{
"firstList": {Capacity: 30},
"firstList": {Capacity: 30},
"secondList": {Capacity: 100, Values: []string{"4"}},
}
list = append(list, gs1, gs2, gs3, gs4)

Expand All @@ -419,12 +421,12 @@ func TestComputeStatus(t *testing.T) {
Counters: map[string]agonesv1.AggregatedCounterStatus{},
Lists: map[string]agonesv1.AggregatedListStatus{
"firstList": {
Capacity: 85,
Values: []string{"a", "b", "c", "d"},
Capacity: 55,
Values: []string{"a", "b", "c"},
},
"secondList": {
Capacity: 2000,
Values: []string{"1", "2", "3"},
Capacity: 1100,
Values: []string{"1", "2", "4"},
},
},
}
Expand Down

0 comments on commit 84c2677

Please sign in to comment.