Add Priorities to GameServerSet

install/helm/agones/templates/crds/fleet.yaml

@@ -110,7 +110,7 @@ spec:
                           anyOf:
                             - type: integer
                             - type: string
-                priorities: # which gameservers in the Fleet are most important to keep around - impacts scale down logic
+                priorities: # which gameservers in the Fleet are most important to keep around - impacts scale down logic. Must be identical to priorities in gameserverset.yaml.
                   type: array
                   title: Configuration of Counters and Lists scale down logic
                   nullable: true

install/helm/agones/templates/crds/gameserverset.yaml

@@ -90,6 +90,22 @@ spec:
                   enum:
                     - Packed
                     - Distributed
+                priorities: # see fleet.yaml
+                  type: array
+                  title: Configuration of Counters and Lists scale down logic
+                  nullable: true
+                  items:
+                    type: object
+                    properties:
+                      priorityType:
+                        type: string
+                        title: Whether a Counter or a List.
+                      key:
+                        type: string
+                        title: The name of the Counter or List
+                      order:
+                        type: string
+                        title: Ascending or Descending sort order
                 template:
                   {{- $data := dict "metadata" true "podPreserveUnknownFields" .Values.gameservers.podPreserveUnknownFields }}
                   {{- include "gameserver.schema" $data | indent 18 }}

pkg/apis/agones/v1/fleet.go

@@ -137,6 +137,11 @@ func (f *Fleet) GameServerSet() *GameServerSet {
 		gsSet.Spec.AllocationOverflow = f.Spec.AllocationOverflow.DeepCopy()
 	}
 
+	// TODO: Do we need a DeepCopy here? (Priorities does not have a method DeepCopy) Priorities should not change (unless changes are made directly to the fleet CRD yaml and the fleet re-applied at which point this field would get update anyways)
+	if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && f.Spec.Priorities != nil {
+		gsSet.Spec.Priorities = f.Spec.Priorities
+	}
+
 	return gsSet
 }
 

pkg/apis/agones/v1/fleet_test.go

@@ -77,17 +77,19 @@ func TestFleetGameServerSetGameServer(t *testing.T) {
 		Annotations: nil,
 	}
 
-	gsSet = f.GameServerSet()
-	assert.NotNil(t, gsSet.Spec.AllocationOverflow)
-	assert.Equal(t, "things", gsSet.Spec.AllocationOverflow.Labels["stuff"])
-
 	assert.Nil(t, f.Spec.Priorities)
 	f.Spec.Priorities = []Priority{
 		{PriorityType: "Counter",
 			Key:   "Foo",
 			Order: "Ascending"}}
 	assert.NotNil(t, f.Spec.Priorities)
 	assert.Equal(t, f.Spec.Priorities[0], Priority{PriorityType: "Counter", Key: "Foo", Order: "Ascending"})
+
+	gsSet = f.GameServerSet()
+	assert.NotNil(t, gsSet.Spec.AllocationOverflow)
+	assert.Equal(t, "things", gsSet.Spec.AllocationOverflow.Labels["stuff"])
+
+	assert.Equal(t, gsSet.Spec.Priorities[0], Priority{PriorityType: "Counter", Key: "Foo", Order: "Ascending"})
 }
 
 func TestFleetApplyDefaults(t *testing.T) {
@@ -311,6 +313,7 @@ func TestGetReadyReplicaCountForGameServerSets(t *testing.T) {
 	assert.Equal(t, int32(1020), GetReadyReplicaCountForGameServerSets(fixture))
 }
 
+// nolint:dupl  // Linter errors on lines are duplicate of TestGameServerSetPriorityValidate
 func TestFleetPriorityValidate(t *testing.T) {
 	t.Parallel()
 

pkg/apis/agones/v1/gameserverset.go

@@ -15,8 +15,11 @@
 package v1
 
 import (
+	"fmt"
+
 	"agones.dev/agones/pkg/apis"
 	"agones.dev/agones/pkg/apis/agones"
+	"agones.dev/agones/pkg/util/runtime"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -65,6 +68,12 @@ type GameServerSetSpec struct {
 	AllocationOverflow *AllocationOverflow `json:"allocationOverflow,omitempty"`
 	// Scheduling strategy. Defaults to "Packed".
 	Scheduling apis.SchedulingStrategy `json:"scheduling,omitempty"`
+	// (Alpha, CountsAndLists feature flag) The first Priority on the array of Priorities is the most
+	// important for sorting. The Fleetautoscaler will use the first priority for sorting GameServers
+	// by total Capacity in the Fleet and acts as a tie-breaker after sorting the game servers by
+	// State and Strategy. Impacts scale down logic.
+	// +optional
+	Priorities []Priority `json:"priorities,omitempty"`
 	// Template the GameServer template to apply for this GameServerSet
 	Template GameServerTemplateSpec `json:"template"`
 }
@@ -128,6 +137,22 @@ func (gsSet *GameServerSet) Validate(apiHooks APIHooks) ([]metav1.StatusCause, b
 		causes = append(causes, objMetaCauses...)
 	}
 
+	if !runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && (gsSet.Spec.Priorities != nil) {
+		causes = append(causes, metav1.StatusCause{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Feature CountsAndLists must be enabled if Priorities is specified",
+			Field:   "spec.priorities",
+		})
+	}
+
+	if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) && (gsSet.Spec.Priorities != nil) {
+		for i := range gsSet.Spec.Priorities {
+			if c, ok := gsSet.Spec.Priorities[i].Validate(fmt.Sprintf("spec.priorities[%d]", i)); !ok {
+				causes = append(causes, c...)
+			}
+		}
+	}
+
 	return causes, len(causes) == 0
 }
 

pkg/apis/agones/v1/gameserverset_test.go

@@ -15,9 +15,11 @@
 package v1
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
+	"agones.dev/agones/pkg/util/runtime"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -152,8 +154,182 @@ func TestGameServerSetValidateUpdate(t *testing.T) {
 			},
 		}
 	causes, ok = gsSet.Validate(fakeAPIHooks{})
-
 	assert.False(t, ok)
 	assert.Len(t, causes, 2)
 	assert.Equal(t, "container", causes[0].Field)
+
+	// Priority Updates
+	newGSS = gsSet.DeepCopy()
+	newGSS.Spec.Priorities = []Priority{
+		{PriorityType: "List",
+			Key:   "Bar",
+			Order: "Descending"}}
+	causes, ok = gsSet.ValidateUpdate(newGSS)
+	assert.True(t, ok)
+	assert.Empty(t, causes)
+}
+
+// nolint:dupl  // Linter errors on lines are duplicate of TestFleetPriorityValidate
+func TestGameServerSetPriorityValidate(t *testing.T) {
+	t.Parallel()
+
+	runtime.FeatureTestMutex.Lock()
+	defer runtime.FeatureTestMutex.Unlock()
+	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists)))
+
+	modifiedGsSet := func(f func(*GameServerSetSpec)) *GameServerSet {
+		gsSet := defaultGsSet()
+		f(&gsSet.Spec)
+		return gsSet
+	}
+
+	type expected struct {
+		valid    bool
+		causeLen int
+		fields   []string
+	}
+
+	fixtures := map[string]struct {
+		gsSet    *GameServerSet
+		expected expected
+	}{
+		"valid Counter Ascending": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "Counter",
+						Key:          "Foo",
+						Order:        "Ascending",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"valid Counter Descending": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "Counter",
+						Key:          "Bar",
+						Order:        "Descending",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"valid Counter empty Order": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "Counter",
+						Key:          "Bar",
+						Order:        "",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"invalid counter type and order": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "counter",
+						Key:          "Babar",
+						Order:        "descending",
+					}}
+			}),
+			expected: expected{
+				valid:    false,
+				causeLen: 2,
+			},
+		},
+		"valid List Ascending": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "List",
+						Key:          "Baz",
+						Order:        "Ascending",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"valid List Descending": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "List",
+						Key:          "Blerg",
+						Order:        "Descending",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"valid List empty Order": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "List",
+						Key:          "Blerg",
+						Order:        "Ascending",
+					}}
+			}),
+			expected: expected{
+				valid:    true,
+				causeLen: 0,
+			},
+		},
+		"invalid list type and order": {
+			gsSet: modifiedGsSet(func(gsss *GameServerSetSpec) {
+				gsss.Priorities = []Priority{
+					{
+						PriorityType: "list",
+						Key:          "Schmorg",
+						Order:        "ascending",
+					}}
+			}),
+			expected: expected{
+				valid:    false,
+				causeLen: 2,
+			},
+		},
+	}
+
+	for k, v := range fixtures {
+		t.Run(k, func(t *testing.T) {
+			causes, valid := v.gsSet.Validate(fakeAPIHooks{})
+			assert.Equal(t, v.expected.valid, valid)
+			assert.Len(t, causes, v.expected.causeLen)
+
+			for i := range v.expected.fields {
+				assert.Equal(t, v.expected.fields[i], causes[i].Field)
+			}
+		})
+	}
+}
+
+func defaultGsSet() *GameServerSet {
+	gsSpec := defaultGameServer().Spec
+	gsSet := GameServerSet{
+		ObjectMeta: metav1.ObjectMeta{Name: "test"},
+		Spec: GameServerSetSpec{
+			Replicas: 10,
+			Template: GameServerTemplateSpec{
+				Spec: gsSpec,
+			},
+		},
+	}
+	return &gsSet
 }

pkg/fleets/controller.go

@@ -31,6 +31,7 @@ import (
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/webhooks"
 	"agones.dev/agones/pkg/util/workerqueue"
+	"github.com/google/go-cmp/cmp"
 	"github.com/heptiolabs/healthcheck"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/pkg/errors"
@@ -364,6 +365,20 @@ func (c *Controller) upsertGameServerSet(ctx context.Context, fleet *agonesv1.Fl
 			"Scaling active GameServerSet %s from %d to %d", gsSetCopy.ObjectMeta.Name, active.Spec.Replicas, gsSetCopy.Spec.Replicas)
 	}
 
+	// Update GameServerSet Counts and Lists Priorities if not equal to the Priorities on the Fleet
+	if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) {
+		if !cmp.Equal(active.Spec.Priorities, fleet.Spec.Priorities) {
+			gsSetCopy := active.DeepCopy()
+			gsSetCopy.Spec.Priorities = fleet.Spec.Priorities
+			_, err := c.gameServerSetGetter.GameServerSets(fleet.ObjectMeta.Namespace).Update(ctx, gsSetCopy, metav1.UpdateOptions{})
+			if err != nil {
+				return errors.Wrapf(err, "error updating priorities for gameserverset for fleet %s", fleet.ObjectMeta.Name)
+			}
+			c.recorder.Eventf(fleet, corev1.EventTypeNormal, "UpdatingGameServerSet",
+				"Updated GameServerSet %s Priorities", gsSetCopy.ObjectMeta.Name)
+		}
+	}
+
 	return nil
 }
 

pkg/fleets/controller_test.go

@@ -1063,6 +1063,40 @@ func TestControllerUpsertGameServerSet(t *testing.T) {
 		assert.Nil(t, err)
 		agtesting.AssertNoEvent(t, m.FakeRecorder.Events)
 	})
+
+	t.Run("update Priorities", func(t *testing.T) {
+		utilruntime.FeatureTestMutex.Lock()
+		defer utilruntime.FeatureTestMutex.Unlock()
+		require.NoError(t, utilruntime.ParseFeatures(string(utilruntime.FeatureCountsAndLists)+"=true"))
+
+		c, m := newFakeController()
+		// Default GameServerSet has no Priorities
+		gsSet := f.GameServerSet()
+		gsSet.ObjectMeta.UID = "1234"
+		// Add Priorities to the Fleet
+		f.Spec.Priorities = []agonesv1.Priority{
+			{
+				PriorityType: "List",
+				Key:          "Baz",
+				Order:        "Ascending",
+			}}
+		update := false
+
+		m.AgonesClient.AddReactor("update", "gameserversets", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			update = true
+			ca := action.(k8stesting.UpdateAction)
+			gsSet := ca.GetObject().(*agonesv1.GameServerSet)
+			assert.Equal(t, agonesv1.Priority{PriorityType: "List", Key: "Baz", Order: "Ascending"}, gsSet.Spec.Priorities[0])
+			return true, gsSet, nil
+		})
+
+		// Update Priorities on the GameServerSet to match the Fleet
+		err := c.upsertGameServerSet(context.Background(), f, gsSet, gsSet.Spec.Replicas)
+		assert.Nil(t, err)
+
+		assert.True(t, update, "Should be updated")
+		agtesting.AssertEventContains(t, m.FakeRecorder.Events, "UpdatingGameServerSet")
+	})
 }
 
 func TestResourcesRequestsAndLimits(t *testing.T) {