Graduate StateAllocationFilter to Stable (#3308)

* Graduate StateAllocationFilter to Stable

* resolved lint

* revised necessary modification

* GameServerState update

* modified expected values in find_test.go

* one label test deleted

* make gen-api-docs target made change in aones_crd_api_reference.html

* Alter shortcode content

* Elimination of FeatureStateAllocationFilter

* removed log for state allocation filter

cloudbuild.yaml

@@ -325,10 +325,10 @@ steps:
           region=${versionsAndRegions[$version]}
           if [ $cloudProduct = generic ]
           then
-            featureWithGate="StateAllocationFilter=false&PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&PodHostname=false&SplitControllerAndExtensions=false&FleetAllocationOverflow=true&Example=true"
+            featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&PodHostname=false&SplitControllerAndExtensions=false&FleetAllocationOverflow=true&Example=true"
             testCluster="standard-e2e-test-cluster-${version//./-}"
           else
-            featureWithGate="StateAllocationFilter=false&PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&PodHostname=false&SplitControllerAndExtensions=true&FleetAllocationOverflow=true&Example=true"
+            featureWithGate="PlayerAllocationFilter=true&PlayerTracking=true&ResetMetricsOnDelete=false&PodHostname=false&SplitControllerAndExtensions=true&FleetAllocationOverflow=true&Example=true"
             testCluster="gke-autopilot-e2e-test-cluster-${version//./-}"
           fi
           featureWithoutGate=""

examples/gameserverallocation-deprecated.yaml

@@ -37,11 +37,9 @@ spec:
       game: my-game
     matchExpressions:
       - {key: tier, operator: In, values: [cache]}
-    # [Stage:Beta]
-    # [FeatureFlag:StateAllocationFilter]
     # Specifies which State is the filter to be used when attempting to retrieve a GameServer
     # via Allocation. Defaults to "Ready". The only other option is "Allocated", which can be used in conjunction with
-    # label/annotation/player selectors to retrieve an already Allocated GameServer.
+    # label/annotation/player selectors to retrieve an already Allocated GameServer.  
     gameServerState: Ready
     # [Stage:Alpha]
     # [FeatureFlag:PlayerAllocationFilter]

examples/gameserverallocation.yaml

@@ -46,8 +46,6 @@ spec:
         game: my-game
       matchExpressions:
       - {key: tier, operator: In, values: [cache]}
-      # [Stage:Beta]
-      # [FeatureFlag:StateAllocationFilter]
       # Specifies which State is the filter to be used when attempting to retrieve a GameServer
       # via Allocation. Defaults to "Ready". The only other option is "Allocated", which can be used in conjunction with
       # label/annotation/player selectors to retrieve an already Allocated GameServer.

install/helm/agones/defaultfeaturegates.yaml

@@ -18,7 +18,6 @@
 PodHostname: true
 ResetMetricsOnDelete: true
 SplitControllerAndExtensions: true
-StateAllocationFilter: true
 
 # Alpha features
 PlayerAllocationFilter: false

pkg/allocation/converters/converter.go

@@ -164,15 +164,13 @@ func convertGameServerSelectorToInternalGameServerSelector(in *pb.GameServerSele
 		LabelSelector: metav1.LabelSelector{MatchLabels: in.GetMatchLabels()},
 	}
 
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-		switch in.GameServerState {
-		case pb.GameServerSelector_ALLOCATED:
-			allocated := agonesv1.GameServerStateAllocated
-			result.GameServerState = &allocated
-		case pb.GameServerSelector_READY:
-			ready := agonesv1.GameServerStateReady
-			result.GameServerState = &ready
-		}
+	switch in.GameServerState {
+	case pb.GameServerSelector_ALLOCATED:
+		allocated := agonesv1.GameServerStateAllocated
+		result.GameServerState = &allocated
+	case pb.GameServerSelector_READY:
+		ready := agonesv1.GameServerStateReady
+		result.GameServerState = &ready
 	}
 
 	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) && in.Players != nil {
@@ -217,7 +215,7 @@ func convertInternalGameServerSelectorToGameServer(in *allocationv1.GameServerSe
 		MatchLabels: in.MatchLabels,
 	}
 
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) && in.GameServerState != nil {
+	if in.GameServerState != nil {
 		switch *in.GameServerState {
 		case agonesv1.GameServerStateReady:
 			result.GameServerState = pb.GameServerSelector_READY

pkg/allocation/converters/converter_test.go

@@ -42,8 +42,8 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 		want     *allocationv1.GameServerAllocation
 	}{
 		{
-			name:     "all fields are set (StateAllocationFilter, PlayerAllocationFilter, CountsAndListsFilter)",
-			features: fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			name:     "all fields are set (PlayerAllocationFilter, CountsAndListsFilter)",
+			features: fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace: "ns",
 				MultiClusterSetting: &pb.MultiClusterSetting{
@@ -217,7 +217,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 		},
 		{
 			name:     "all fields are set",
-			features: fmt.Sprintf("%s=false&%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			features: fmt.Sprintf("%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace: "ns",
 				MultiClusterSetting: &pb.MultiClusterSetting{
@@ -281,21 +281,25 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 						MatchLabels: map[string]string{
 							"c": "d",
 						},
-					}},
+					},
+						GameServerState: &ready,
+					},
 					Preferred: []allocationv1.GameServerSelector{
 						{
 							LabelSelector: metav1.LabelSelector{
 								MatchLabels: map[string]string{
 									"e": "f",
 								},
 							},
+							GameServerState: &ready,
 						},
 						{
 							LabelSelector: metav1.LabelSelector{
 								MatchLabels: map[string]string{
 									"g": "h",
 								},
 							},
+							GameServerState: &ready,
 						},
 					},
 					Selectors: []allocationv1.GameServerSelector{
@@ -305,6 +309,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 									"m": "n",
 								},
 							},
+							GameServerState: &ready,
 						},
 					},
 					Scheduling: apis.Packed,
@@ -318,7 +323,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 		},
 		{
 			name:     "empty fields to GSA",
-			features: fmt.Sprintf("%s=false&%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			features: fmt.Sprintf("%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace:                    "",
 				MultiClusterSetting:          &pb.MultiClusterSetting{},
@@ -336,12 +341,15 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 						Enabled: false,
 					},
 					Scheduling: apis.Distributed,
+					Required: allocationv1.GameServerSelector{
+						GameServerState: &ready,
+					},
 				},
 			},
 		},
 		{
-			name:     "empty fields to GSA (StateAllocationFilter, PlayerAllocationFilter, CountsAndListsFilter)",
-			features: fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			name:     "empty fields to GSA (PlayerAllocationFilter, CountsAndListsFilter)",
+			features: fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace:                    "",
 				MultiClusterSetting:          &pb.MultiClusterSetting{},
@@ -367,7 +375,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 		},
 		{
 			name:     "empty fields to GSA with selectors fields",
-			features: fmt.Sprintf("%s=false&%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			features: fmt.Sprintf("%s=false&%s=false", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace:           "",
 				MultiClusterSetting: &pb.MultiClusterSetting{},
@@ -383,14 +391,16 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					MultiClusterSetting: allocationv1.MultiClusterSetting{
 						Enabled: false,
 					},
-					Selectors:  []allocationv1.GameServerSelector{{}},
+					Selectors: []allocationv1.GameServerSelector{{
+						GameServerState: &ready,
+					}},
 					Scheduling: apis.Distributed,
 				},
 			},
 		},
 		{
-			name:     "empty fields to GSA (StateAllocationFilter, PlayerAllocationFilter, CountsAndListsFilter) with selectors fields",
-			features: fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			name:     "empty fields to GSA (PlayerAllocationFilter, CountsAndListsFilter) with selectors fields",
+			features: fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace:           "",
 				MultiClusterSetting: &pb.MultiClusterSetting{},
@@ -501,8 +511,8 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 			},
 		},
 		{
-			name:     "partially empty Counters and Lists fields to GSA (StateAllocationFilter, PlayerAllocationFilter, CountsAndListsFilter)",
-			features: fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists),
+			name:     "partially empty Counters and Lists fields to GSA (PlayerAllocationFilter, CountsAndListsFilter)",
+			features: fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists),
 			in: &pb.AllocationRequest{
 				Namespace:                    "",
 				MultiClusterSetting:          &pb.MultiClusterSetting{},

pkg/apis/allocation/v1/gameserverallocation.go

@@ -117,9 +117,6 @@ type GameServerAllocationSpec struct {
 type GameServerSelector struct {
 	// See: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
 	metav1.LabelSelector `json:",inline"`
-	// [Stage:Beta]
-	// [FeatureFlag:StateAllocationFilter]
-	// +optional
 	// GameServerState specifies which State is the filter to be used when attempting to retrieve a GameServer
 	// via Allocation. Defaults to "Ready". The only other option is "Allocated", which can be used in conjunction with
 	// label/annotation/player selectors to retrieve an already Allocated GameServer.
@@ -187,11 +184,9 @@ type ListAction struct {
 
 // ApplyDefaults applies default values
 func (s *GameServerSelector) ApplyDefaults() {
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-		if s.GameServerState == nil {
-			state := agonesv1.GameServerStateReady
-			s.GameServerState = &state
-		}
+	if s.GameServerState == nil {
+		state := agonesv1.GameServerStateReady
+		s.GameServerState = &state
 	}
 
 	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) {
@@ -229,10 +224,8 @@ func (s *GameServerSelector) Matches(gs *agonesv1.GameServer) bool {
 	}
 
 	// then if state is being checked, check state
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-		if s.GameServerState != nil && gs.Status.State != *s.GameServerState {
-			return false
-		}
+	if s.GameServerState != nil && gs.Status.State != *s.GameServerState {
+		return false
 	}
 
 	// then if player count is being checked, check that
@@ -371,10 +364,8 @@ func (s *GameServerSelector) Validate(fldPath *field.Path) field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("labelSelector"), s.LabelSelector, fmt.Sprintf("Error converting label selector: %s", err)))
 	}
 
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-		if s.GameServerState != nil && !(*s.GameServerState == agonesv1.GameServerStateAllocated || *s.GameServerState == agonesv1.GameServerStateReady) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("gameServerState"), *s.GameServerState, "GameServerState must be either Allocated or Ready"))
-		}
+	if s.GameServerState != nil && !(*s.GameServerState == agonesv1.GameServerStateAllocated || *s.GameServerState == agonesv1.GameServerStateReady) {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("gameServerState"), *s.GameServerState, "GameServerState must be either Allocated or Ready"))
 	}
 
 	if runtime.FeatureEnabled(runtime.FeaturePlayerAllocationFilter) && s.Players != nil {

pkg/apis/allocation/v1/gameserverallocation_test.go

@@ -41,7 +41,7 @@ func TestGameServerAllocationApplyDefaults(t *testing.T) {
 
 	runtime.FeatureTestMutex.Lock()
 	defer runtime.FeatureTestMutex.Unlock()
-	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists)))
+	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists)))
 
 	gsa = &GameServerAllocation{}
 	gsa.ApplyDefaults()
@@ -114,9 +114,8 @@ func TestGameServerSelectorApplyDefaults(t *testing.T) {
 	runtime.FeatureTestMutex.Lock()
 	defer runtime.FeatureTestMutex.Unlock()
 
-	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true&%s=true",
+	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true",
 		runtime.FeaturePlayerAllocationFilter,
-		runtime.FeatureStateAllocationFilter,
 		runtime.FeatureCountsAndLists)))
 
 	s := &GameServerSelector{}
@@ -156,7 +155,7 @@ func TestGameServerSelectorValidate(t *testing.T) {
 	runtime.FeatureTestMutex.Lock()
 	defer runtime.FeatureTestMutex.Unlock()
 
-	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter, runtime.FeatureCountsAndLists)))
+	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureCountsAndLists)))
 
 	allocated := agonesv1.GameServerStateAllocated
 	starting := agonesv1.GameServerStateStarting
@@ -441,15 +440,13 @@ func TestGameServerSelectorMatches(t *testing.T) {
 			matches: false,
 		},
 		"state filter, pass": {
-			features: string(runtime.FeatureStateAllocationFilter) + "=true",
 			selector: &GameServerSelector{
 				GameServerState: &allocatedState,
 			},
 			gameServer: &agonesv1.GameServer{Status: agonesv1.GameServerStatus{State: allocatedState}},
 			matches:    true,
 		},
 		"state filter, fail": {
-			features: string(runtime.FeatureStateAllocationFilter) + "=true",
 			selector: &GameServerSelector{
 				GameServerState: &allocatedState,
 			},
@@ -511,7 +508,7 @@ func TestGameServerSelectorMatches(t *testing.T) {
 			matches: true,
 		},
 		"combo": {
-			features: string(runtime.FeaturePlayerAllocationFilter) + "=true&" + string(runtime.FeatureStateAllocationFilter) + "=true",
+			features: string(runtime.FeaturePlayerAllocationFilter) + "=true&",
 			selector: &GameServerSelector{
 				LabelSelector: metav1.LabelSelector{
 					MatchLabels: map[string]string{"colour": "blue"},
@@ -1060,7 +1057,7 @@ func TestGameServerAllocationValidate(t *testing.T) {
 
 	runtime.FeatureTestMutex.Lock()
 	defer runtime.FeatureTestMutex.Unlock()
-	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter)))
+	assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeaturePlayerAllocationFilter)))
 
 	// invalid player selection
 	gsa = &GameServerAllocation{

pkg/gameserverallocations/allocation_cache.go

@@ -36,11 +36,6 @@ import (
 
 type matcher func(*agonesv1.GameServer) bool
 
-// readyGameServerMatcher return true when a GameServer is in a Ready state.
-func readyGameServerMatcher(gs *agonesv1.GameServer) bool {
-	return gs.Status.State == agonesv1.GameServerStateReady
-}
-
 // readyOrAllocatedGameServerMatcher returns true when a GameServer is in a Ready or Allocated state.
 func readyOrAllocatedGameServerMatcher(gs *agonesv1.GameServer) bool {
 	return gs.Status.State == agonesv1.GameServerStateReady || gs.Status.State == agonesv1.GameServerStateAllocated
@@ -63,12 +58,7 @@ func NewAllocationCache(informer informerv1.GameServerInformer, counter *gameser
 		gameServerSynced: informer.Informer().HasSynced,
 		gameServerLister: informer.Lister(),
 		counter:          counter,
-		matcher:          readyGameServerMatcher,
-	}
-
-	// if we can do state filtering, then cache both Ready and Allocated GameServers
-	if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-		c.matcher = readyOrAllocatedGameServerMatcher
+		matcher:          readyOrAllocatedGameServerMatcher,
 	}
 
 	_, _ = informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
@@ -184,11 +174,9 @@ func (c *AllocationCache) ListSortedGameServers(gsa *allocationv1.GameServerAllo
 		gs1 := list[i]
 		gs2 := list[j]
 
-		if runtime.FeatureEnabled(runtime.FeatureStateAllocationFilter) {
-			// Search Allocated GameServers first.
-			if gs1.Status.State != gs2.Status.State {
-				return gs1.Status.State == agonesv1.GameServerStateAllocated
-			}
+		// Search Allocated GameServers first.
+		if gs1.Status.State != gs2.Status.State {
+			return gs1.Status.State == agonesv1.GameServerStateAllocated
 		}
 
 		c1, ok := counts[gs1.Status.NodeName]