GSA: Switch LabelSelector to GameServerSelector

pkg/allocation/converters/converter.go

@@ -64,7 +64,7 @@ func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) *allocationv1.GameS
 	}
 
 	if ls := convertLabelSelectorToInternalLabelSelector(in.GetRequiredGameServerSelector()); ls != nil {
-		gsa.Spec.Required = *ls
+		gsa.Spec.Required = allocationv1.GameServerSelector{LabelSelector: *ls}
 	}
 	return gsa
 }
@@ -82,7 +82,7 @@ func ConvertGSAToAllocationRequest(in *allocationv1.GameServerAllocation) *pb.Al
 		MultiClusterSetting: &pb.MultiClusterSetting{
 			Enabled: in.Spec.MultiClusterSetting.Enabled,
 		},
-		RequiredGameServerSelector: convertInternalLabelSelectorToLabelSelector(&in.Spec.Required),
+		RequiredGameServerSelector: convertInternalLabelSelectorToLabelSelector(&in.Spec.Required.LabelSelector),
 		Metadata: &pb.MetaPatch{
 			Labels:      in.Spec.MetaPatch.Labels,
 			Annotations: in.Spec.MetaPatch.Annotations,
@@ -139,21 +139,21 @@ func convertInternalLabelSelectorToLabelSelector(in *metav1.LabelSelector) *pb.L
 	return &pb.LabelSelector{MatchLabels: in.MatchLabels}
 }
 
-func convertInternalLabelSelectorsToLabelSelectors(in []metav1.LabelSelector) []*pb.LabelSelector {
+func convertInternalLabelSelectorsToLabelSelectors(in []allocationv1.GameServerSelector) []*pb.LabelSelector {
 	var result []*pb.LabelSelector
 	for _, l := range in {
 		l := l
-		c := convertInternalLabelSelectorToLabelSelector(&l)
+		c := convertInternalLabelSelectorToLabelSelector(&l.LabelSelector)
 		result = append(result, c)
 	}
 	return result
 }
 
-func convertLabelSelectorsToInternalLabelSelectors(in []*pb.LabelSelector) []metav1.LabelSelector {
-	var result []metav1.LabelSelector
+func convertLabelSelectorsToInternalLabelSelectors(in []*pb.LabelSelector) []allocationv1.GameServerSelector {
+	var result []allocationv1.GameServerSelector
 	for _, l := range in {
 		if c := convertLabelSelectorToInternalLabelSelector(l); c != nil {
-			result = append(result, *c)
+			result = append(result, allocationv1.GameServerSelector{LabelSelector: *c})
 		}
 	}
 	return result

pkg/allocation/converters/converter_test.go

@@ -88,22 +88,25 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 							},
 						},
 					},
-					Required: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"c": "d",
-						},
-					},
-					Preferred: []metav1.LabelSelector{
-						{
+					Required: allocationv1.GameServerSelector{
+						LabelSelector: metav1.LabelSelector{
 							MatchLabels: map[string]string{
-								"e": "f",
+								"c": "d",
 							},
-						},
+						}},
+					Preferred: []allocationv1.GameServerSelector{
 						{
-							MatchLabels: map[string]string{
-								"g": "h",
-							},
-						},
+							LabelSelector: metav1.LabelSelector{
+								MatchLabels: map[string]string{
+									"e": "f",
+								},
+							}},
+						{
+							LabelSelector: metav1.LabelSelector{
+								MatchLabels: map[string]string{
+									"g": "h",
+								},
+							}},
 					},
 					Scheduling: apis.Packed,
 					MetaPatch: allocationv1.MetaPatch{

pkg/apis/allocation/v1/gameserverallocation.go

@@ -19,7 +19,7 @@ import (
 
 	"agones.dev/agones/pkg/apis"
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
-	"github.com/pkg/errors"
+	"agones.dev/agones/pkg/util/runtime"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 )
@@ -67,12 +67,12 @@ type GameServerAllocationSpec struct {
 	MultiClusterSetting MultiClusterSetting `json:"multiClusterSetting,omitempty"`
 
 	// Required The required allocation. Defaults to all GameServers.
-	Required metav1.LabelSelector `json:"required,omitempty"`
+	Required GameServerSelector `json:"required,omitempty"`
 
 	// Preferred ordered list of preferred allocations out of the `required` set.
 	// If the first selector is not matched,
 	// the selection attempts the second selector, and so on.
-	Preferred []metav1.LabelSelector `json:"preferred,omitempty"`
+	Preferred []GameServerSelector `json:"preferred,omitempty"`
 
 	// Scheduling strategy. Defaults to "Packed".
 	Scheduling apis.SchedulingStrategy `json:"scheduling"`
@@ -82,6 +82,133 @@ type GameServerAllocationSpec struct {
 	MetaPatch MetaPatch `json:"metadata,omitempty"`
 }
 
+// GameServerSelector contains all the filter options for selecting
+// a GameServer for allocation.
+type GameServerSelector struct {
+	metav1.LabelSelector
+	// [Stage:Alpha]
+	// [FeatureFlag:StateAllocationFilter]
+	// +optional
+	GameServerState *agonesv1.GameServerState `json:"gameServerState,omitempty"`
+	// [Stage:Alpha]
+	// [FeatureFlag:PlayerAllocationFilter]
+	// +optional
+	Players *PlayerSelector `json:"players,omitempty"`
+}
+
+// PlayerSelector is the filter options for a GameServer based on player counts
+type PlayerSelector struct {
+	MinAvailable int64 `json:"minAvailable,omitempty"`
+	MaxAvailable int64 `json:"maxAvailable,omitempty"`
+}
+
+// ApplyDefaults applies default values to the PlayerSelector
+func (s *GameServerSelector) ApplyDefaults() {
+	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
+		if s.GameServerState == nil {
+			state := agonesv1.GameServerStateReady
+			s.GameServerState = &state
+		}
+	}
+
+	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) {
+		if s.Players == nil {
+			s.Players = &PlayerSelector{}
+		}
+	}
+}
+
+// Matches checks to see if a GameServer matches a given GameServerSelector's criteria.
+// Will panic if the `GameServerSelector` has not passed `Validate()`.
+func (s *GameServerSelector) Matches(gs *agonesv1.GameServer) bool {
+
+	// Assume at this point, this has already been run through Validate(), and it can be converted.
+	// We end up running LabelSelectorAsSelector twice for each allocation, but if we store the results of this
+	// function within the GameServerSelector, we can't fuzz the GameServerAllocation as reflect.DeepEqual
+	// will fail due to the unexported field.
+	selector, err := metav1.LabelSelectorAsSelector(&s.LabelSelector)
+	if err != nil {
+		panic("GameServerSelector.Validate() has not been called before calling GameServerSelector.Matches(...)")
+	}
+
+	// first check labels
+	if !selector.Matches(labels.Set(gs.ObjectMeta.Labels)) {
+		return false
+	}
+
+	// then if state is being checked, check state
+	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
+		if s.GameServerState != nil && gs.Status.State != *s.GameServerState {
+			return false
+		}
+	}
+
+	// then if player count is being checked, check that
+	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) {
+		// 0 is unlimited number of players
+		if s.Players != nil && gs.Status.Players != nil && s.Players.MaxAvailable != 0 {
+			available := gs.Status.Players.Capacity - gs.Status.Players.Count
+			if !(available >= s.Players.MinAvailable && available <= s.Players.MaxAvailable) {
+				return false
+			}
+		}
+	}
+
+	return true
+}
+
+// Validate validates that the selection fields have valid values
+func (s *GameServerSelector) Validate(field string) ([]metav1.StatusCause, bool) {
+	var causes []metav1.StatusCause
+
+	_, err := metav1.LabelSelectorAsSelector(&s.LabelSelector)
+	if err != nil {
+		causes = append(causes, metav1.StatusCause{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: fmt.Sprintf("Error converting label selector: %s", err),
+			Field:   field,
+		})
+	}
+
+	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
+		if s.GameServerState != nil && !(*s.GameServerState == agonesv1.GameServerStateAllocated || *s.GameServerState == agonesv1.GameServerStateReady) {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "GameServerState value can only be Allocated or Ready",
+				Field:   field,
+			})
+		}
+	}
+
+	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) && s.Players != nil {
+		if s.Players.MinAvailable < 0 {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "Players.MinAvailable must be greater than zero",
+				Field:   field,
+			})
+		}
+
+		if s.Players.MaxAvailable < 0 {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "Players.MaxAvailable must be greater than zero",
+				Field:   field,
+			})
+		}
+
+		if s.Players.MinAvailable > s.Players.MaxAvailable {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "Players.MinAvailable must be less than Players.MaxAvailable",
+				Field:   field,
+			})
+		}
+	}
+
+	return causes, len(causes) == 0
+}
+
 // MultiClusterSetting specifies settings for multi-cluster allocation.
 type MultiClusterSetting struct {
 	Enabled        bool                 `json:"enabled,omitempty"`
@@ -94,23 +221,6 @@ type MetaPatch struct {
 	Annotations map[string]string `json:"annotations,omitempty"`
 }
 
-// PreferredSelectors converts all the preferred label selectors into an array of
-// labels.Selectors. This is useful as they all have `Match()` functions!
-func (gsas *GameServerAllocationSpec) PreferredSelectors() ([]labels.Selector, error) {
-	list := make([]labels.Selector, len(gsas.Preferred))
-
-	var err error
-	for i, p := range gsas.Preferred {
-		p := p
-		list[i], err = metav1.LabelSelectorAsSelector(&p)
-		if err != nil {
-			break
-		}
-	}
-
-	return list, errors.WithStack(err)
-}
-
 // GameServerAllocationStatus is the status for an GameServerAllocation resource
 type GameServerAllocationStatus struct {
 	// GameServerState is the current state of an GameServerAllocation, e.g. Allocated, or UnAllocated
@@ -126,9 +236,16 @@ func (gsa *GameServerAllocation) ApplyDefaults() {
 	if gsa.Spec.Scheduling == "" {
 		gsa.Spec.Scheduling = apis.Packed
 	}
+
+	gsa.Spec.Required.ApplyDefaults()
+
+	for i := range gsa.Spec.Preferred {
+		gsa.Spec.Preferred[i].ApplyDefaults()
+	}
 }
 
 // Validate validation for the GameServerAllocation
+// Validate should be called before attempting to Match any of the GameServer selectors.
 func (gsa *GameServerAllocation) Validate() ([]metav1.StatusCause, bool) {
 	var causes []metav1.StatusCause
 
@@ -144,5 +261,14 @@ func (gsa *GameServerAllocation) Validate() ([]metav1.StatusCause, bool) {
 			Message: fmt.Sprintf("Invalid value: %s, value must be either Packed or Distributed", gsa.Spec.Scheduling)})
 	}
 
+	if c, ok := gsa.Spec.Required.Validate("spec.required"); !ok {
+		causes = append(causes, c...)
+	}
+	for i := range gsa.Spec.Preferred {
+		if c, ok := gsa.Spec.Preferred[i].Validate(fmt.Sprintf("spec.preferred[%d]", i)); !ok {
+			causes = append(causes, c...)
+		}
+	}
+
 	return causes, len(causes) == 0
 }