Update the GameServerAllocation Specification to remove required/pref…

…erred (googleforgames#2206)
arunnvkumar · Aug 25, 2021 · b188fb7 · b188fb7
1 parent 4d348fc
commit b188fb7
Show file tree

Hide file tree

Showing 22 changed files with 828 additions and 348 deletions.
diff --git a/examples/gameserverallocation-deprecated.yaml b/examples/gameserverallocation-deprecated.yaml
@@ -0,0 +1,76 @@
+# Copyright 2018 Google LLC All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+#
+# Full example of a GameServerAllocation. This is used to allocate
+# A GameServer out of a set of GameServers. This could be a Fleet,
+# multiple Fleets, or a self managed group of GameServers.
+#
+
+#
+# For a full reference and details: https://agones.dev/site/docs/reference/gameserverallocation/
+#
+
+apiVersion: "allocation.agones.dev/v1"
+kind: GameServerAllocation
+spec:
+  # Deprecated, use selectors instead.
+  # GameServer selector from which to choose GameServers from.
+  # GameServers still have the hard requirement to be `Ready` to be allocated from
+  # however we can also make available `matchExpressions` for even greater
+  # flexibility.
+  # Below is an example of a GameServer allocated against a given fleet.
+  # See: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ for more details
+  required:
+    matchLabels:
+      game: my-game
+    matchExpressions:
+      - {key: tier, operator: In, values: [cache]}
+    # [Stage:Alpha]
+    # [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.
+    gameServerState: Ready
+    # [Stage:Alpha]
+    # [FeatureFlag:PlayerAllocationFilter]
+    # Provides a filter on minimum and maximum values for player capacity when retrieving a GameServer
+    # through Allocation. Defaults to no limits.
+    players:
+      minAvailable: 0
+      maxAvailable: 99
+  # Deprecated, use selectors instead.
+  # 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.
+  # This is useful for things like smoke testing of new game servers.
+  # This also support `matchExpressions`
+  preferred:
+    - matchLabels:
+        agones.dev/fleet: green-fleet
+    - matchLabels:
+        agones.dev/fleet: blue-fleet
+  # defines how GameServers are organised across the cluster.
+  # Options include:
+  # "Packed" (default) is aimed at dynamic Kubernetes clusters, such as cloud providers, wherein we want to bin pack
+  # resources
+  # "Distributed" is aimed at static Kubernetes clusters, wherein we want to distribute resources across the entire
+  # cluster
+  scheduling: Packed
+  # Optional custom metadata that is added to the game server at allocation
+  # You can use this to tell the server necessary session data
+  metadata:
+    labels:
+      mode: deathmatch
+    annotations:
+      map:  garden22
diff --git a/examples/gameserverallocation.yaml b/examples/gameserverallocation.yaml
@@ -31,33 +31,32 @@ spec:
   # flexibility.
   # Below is an example of a GameServer allocated against a given fleet.
   # See: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ for more details
-  required:
-    matchLabels:
-      game: my-game
-    matchExpressions:
-      - {key: tier, operator: In, values: [cache]}
-    # [Stage:Alpha]
-    # [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.
-    gameServerState: Ready
-    # [Stage:Alpha]
-    # [FeatureFlag:PlayerAllocationFilter]
-    # Provides a filter on minimum and maximum values for player capacity when retrieving a GameServer
-    # through Allocation. Defaults to no limits.
-    players:
-      minAvailable: 0
-      maxAvailable: 99
-  # ordered list of preferred allocations out of the `required` set.
+  # The selectors is an ordered list of allocations set.
   # 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.
   # This also support `matchExpressions`
-  preferred:
+  selectors:
     - matchLabels:
         agones.dev/fleet: green-fleet
     - matchLabels:
         agones.dev/fleet: blue-fleet
+    - matchLabels:
+        game: my-game
+      matchExpressions:
+      - {key: tier, operator: In, values: [cache]}
+      # [Stage:Alpha]
+      # [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.
+      gameServerState: Ready
+      # [Stage:Alpha]
+      # [FeatureFlag:PlayerAllocationFilter]
+      # Provides a filter on minimum and maximum values for player capacity when retrieving a GameServer
+      # through Allocation. Defaults to no limits.
+      players:
+        minAvailable: 0
+        maxAvailable: 99
   # defines how GameServers are organised across the cluster.
   # Options include:
   # "Packed" (default) is aimed at dynamic Kubernetes clusters, such as cloud providers, wherein we want to bin pack

diff --git a/pkg/allocation/converters/converter.go b/pkg/allocation/converters/converter.go
@@ -37,7 +37,9 @@ func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) *allocationv1.GameS
 			Namespace: in.GetNamespace(),
 		},
 		Spec: allocationv1.GameServerAllocationSpec{
+			// nolint:staticcheck
 			Preferred:  convertGameServerSelectorsToInternalGameServerSelectors(in.GetPreferredGameServerSelectors()),
+			Selectors:  convertGameServerSelectorsToInternalGameServerSelectors(in.GetGameServerSelectors()),
 			Scheduling: convertAllocationSchedulingToGSASchedulingStrategy(in.GetScheduling()),
 		},
 	}
@@ -64,7 +66,9 @@ func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) *allocationv1.GameS
 		}
 	}
 
+	// nolint:staticcheck
 	if selector := convertGameServerSelectorToInternalGameServerSelector(in.GetRequiredGameServerSelector()); selector != nil {
+		// nolint:staticcheck
 		gsa.Spec.Required = *selector
 	}
 	return gsa
@@ -77,13 +81,12 @@ func ConvertGSAToAllocationRequest(in *allocationv1.GameServerAllocation) *pb.Al
 	}
 
 	out := &pb.AllocationRequest{
-		Namespace:                    in.GetNamespace(),
-		PreferredGameServerSelectors: convertInternalLabelSelectorsToLabelSelectors(in.Spec.Preferred),
-		Scheduling:                   convertGSASchedulingStrategyToAllocationScheduling(in.Spec.Scheduling),
+		Namespace:           in.GetNamespace(),
+		Scheduling:          convertGSASchedulingStrategyToAllocationScheduling(in.Spec.Scheduling),
+		GameServerSelectors: convertInternalLabelSelectorsToLabelSelectors(in.Spec.Selectors),
 		MultiClusterSetting: &pb.MultiClusterSetting{
 			Enabled: in.Spec.MultiClusterSetting.Enabled,
 		},
-		RequiredGameServerSelector: convertInternalGameServerSelectorToGameServer(&in.Spec.Required),
 		Metadata: &pb.MetaPatch{
 			Labels:      in.Spec.MetaPatch.Labels,
 			Annotations: in.Spec.MetaPatch.Annotations,
@@ -97,6 +100,14 @@ func ConvertGSAToAllocationRequest(in *allocationv1.GameServerAllocation) *pb.Al
 		},
 	}
 
+	l := len(out.GameServerSelectors)
+	if l > 0 {
+		// nolint:staticcheck
+		out.PreferredGameServerSelectors = out.GameServerSelectors[:l-1]
+		// nolint:staticcheck
+		out.RequiredGameServerSelector = out.GameServerSelectors[l-1]
+	}
+
 	if in.Spec.MultiClusterSetting.Enabled {
 		out.MultiClusterSetting.PolicySelector = convertInternalLabelSelectorToLabelSelector(&in.Spec.MultiClusterSetting.PolicySelector)
 	}

diff --git a/pkg/allocation/converters/converter_test.go b/pkg/allocation/converters/converter_test.go
@@ -35,11 +35,10 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 	ready := agonesv1.GameServerStateReady
 
 	tests := []struct {
-		name               string
-		features           string
-		in                 *pb.AllocationRequest
-		want               *allocationv1.GameServerAllocation
-		skipConvertFromGSA bool
+		name     string
+		features string
+		in       *pb.AllocationRequest
+		want     *allocationv1.GameServerAllocation
 	}{
 		{
 			name:     "all fields are set (StateAllocationFilter, PlayerAllocationFilter)",
@@ -172,6 +171,13 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 						},
 					},
 				},
+				GameServerSelectors: []*pb.GameServerSelector{
+					{
+						MatchLabels: map[string]string{
+							"m": "n",
+						},
+					},
+				},
 				Scheduling: pb.AllocationRequest_Packed,
 				Metadata: &pb.MetaPatch{
 					Labels: map[string]string{
@@ -218,6 +224,15 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 							},
 						},
 					},
+					Selectors: []allocationv1.GameServerSelector{
+						{
+							LabelSelector: metav1.LabelSelector{
+								MatchLabels: map[string]string{
+									"m": "n",
+								},
+							},
+						},
+					},
 					Scheduling: apis.Packed,
 					MetaPatch: allocationv1.MetaPatch{
 						Labels: map[string]string{
@@ -248,7 +263,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					Scheduling: apis.Distributed,
 				},
 			},
-			skipConvertFromGSA: true,
 		},
 		{
 			name:     "empty fields to GSA (StateAllocationFilter, PlayerAllocationFilter)",
@@ -275,7 +289,31 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					Scheduling: apis.Distributed,
 				},
 			},
-			skipConvertFromGSA: true,
+		},
+		{
+			name:     "empty fields to GSA (StateAllocationFilter, PlayerAllocationFilter) with selectors fields",
+			features: fmt.Sprintf("%s=true&%s=true", runtime.FeaturePlayerAllocationFilter, runtime.FeatureStateAllocationFilter),
+			in: &pb.AllocationRequest{
+				Namespace:           "",
+				MultiClusterSetting: &pb.MultiClusterSetting{},
+				GameServerSelectors: []*pb.GameServerSelector{{}},
+				Scheduling:          pb.AllocationRequest_Distributed,
+				Metadata:            &pb.MetaPatch{},
+			},
+			want: &allocationv1.GameServerAllocation{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "",
+				},
+				Spec: allocationv1.GameServerAllocationSpec{
+					MultiClusterSetting: allocationv1.MultiClusterSetting{
+						Enabled: false,
+					},
+					Selectors: []allocationv1.GameServerSelector{
+						{GameServerState: &ready},
+					},
+					Scheduling: apis.Distributed,
+				},
+			},
 		},
 		{
 			name: "empty object to GSA",
@@ -285,7 +323,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					Scheduling: apis.Packed,
 				},
 			},
-			skipConvertFromGSA: true,
 		},
 		{
 			name: "nil object",
@@ -322,7 +359,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					},
 				},
 			},
-			skipConvertFromGSA: true,
 		},
 		{
 			name: "Prefers metadata over metapatch field",
@@ -359,7 +395,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					},
 				},
 			},
-			skipConvertFromGSA: true,
 		},
 	}
 	for _, tc := range tests {
@@ -373,11 +408,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 
 			out := ConvertAllocationRequestToGSA(tc.in)
 			assert.Equal(t, tc.want, out, "mismatch with want after conversion: \"%s\"", tc.name)
-
-			if !tc.skipConvertFromGSA {
-				gsa := ConvertGSAToAllocationRequest(tc.want)
-				assert.Equal(t, tc.in, gsa, "mismatch with input after double conversion \"%s\"", tc.name)
-			}
 		})
 	}
 }
@@ -402,12 +432,11 @@ func TestConvertGSAToAllocationRequestEmpty(t *testing.T) {
 				},
 			},
 			want: &pb.AllocationRequest{
-				Namespace:                  "",
-				MultiClusterSetting:        &pb.MultiClusterSetting{},
-				RequiredGameServerSelector: &pb.GameServerSelector{},
-				Scheduling:                 pb.AllocationRequest_Distributed,
-				Metadata:                   &pb.MetaPatch{},
-				MetaPatch:                  &pb.MetaPatch{},
+				Namespace:           "",
+				MultiClusterSetting: &pb.MultiClusterSetting{},
+				Scheduling:          pb.AllocationRequest_Distributed,
+				Metadata:            &pb.MetaPatch{},
+				MetaPatch:           &pb.MetaPatch{},
 			},
 		}, {
 			name: "empty object",
@@ -417,10 +446,9 @@ func TestConvertGSAToAllocationRequestEmpty(t *testing.T) {
 				},
 			},
 			want: &pb.AllocationRequest{
-				MultiClusterSetting:        &pb.MultiClusterSetting{},
-				RequiredGameServerSelector: &pb.GameServerSelector{},
-				Metadata:                   &pb.MetaPatch{},
-				MetaPatch:                  &pb.MetaPatch{},
+				MultiClusterSetting: &pb.MultiClusterSetting{},
+				Metadata:            &pb.MetaPatch{},
+				MetaPatch:           &pb.MetaPatch{},
 			},
 		},
 	}