Validate the .scheduling field in the fleet and gameserverset

pkg/apis/agones/v1/apihooks.go

@@ -15,6 +15,7 @@
 package v1
 
 import (
+	"agones.dev/agones/pkg/apis"
 	"agones.dev/agones/pkg/util/runtime"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -28,6 +29,9 @@ type APIHooks interface {
 	// ValidateGameServerSpec is called by GameServer.Validate to allow for product specific validation.
 	ValidateGameServerSpec(*GameServerSpec) []metav1.StatusCause
 
+	// ValidateScheduling is called by Fleet and GameServerSet Validate() to allow for product specific validation of scheduling strategy.
+	ValidateScheduling(apis.SchedulingStrategy) []metav1.StatusCause
+
 	// MutateGameServerPodSpec is called by createGameServerPod to allow for product specific pod mutation.
 	MutateGameServerPodSpec(*GameServerSpec, *corev1.PodSpec) error
 
@@ -48,8 +52,9 @@ func RegisterAPIHooks(hooks APIHooks) {
 
 type generic struct{}
 
-func (generic) ValidateGameServerSpec(*GameServerSpec) []metav1.StatusCause    { return nil }
-func (generic) MutateGameServerPodSpec(*GameServerSpec, *corev1.PodSpec) error { return nil }
+func (generic) ValidateGameServerSpec(*GameServerSpec) []metav1.StatusCause     { return nil }
+func (generic) ValidateScheduling(apis.SchedulingStrategy) []metav1.StatusCause { return nil }
+func (generic) MutateGameServerPodSpec(*GameServerSpec, *corev1.PodSpec) error  { return nil }
 
 // SetEviction sets disruptions controls based on GameServer.Status.Eviction.
 func (generic) SetEviction(safe EvictionSafe, pod *corev1.Pod) error {

pkg/apis/agones/v1/fleet.go

@@ -190,6 +190,9 @@ func (f *Fleet) Validate() ([]metav1.StatusCause, bool) {
 	if len(gsCauses) > 0 {
 		causes = append(causes, gsCauses...)
 	}
+	if productCauses := apiHooks.ValidateScheduling(f.Spec.Scheduling); len(productCauses) > 0 {
+		causes = append(causes, productCauses...)
+	}
 	objMetaCauses := validateObjectMeta(&f.Spec.Template.ObjectMeta)
 	if len(objMetaCauses) > 0 {
 		causes = append(causes, objMetaCauses...)

pkg/apis/agones/v1/gameserverset.go

@@ -106,7 +106,9 @@ func (gsSet *GameServerSet) Validate() ([]metav1.StatusCause, bool) {
 	if len(gsCauses) > 0 {
 		causes = append(causes, gsCauses...)
 	}
-
+	if productCauses := apiHooks.ValidateScheduling(gsSet.Spec.Scheduling); len(productCauses) > 0 {
+		causes = append(causes, productCauses...)
+	}
 	objMetaCauses := validateObjectMeta(&gsSet.Spec.Template.ObjectMeta)
 	if len(objMetaCauses) > 0 {
 		causes = append(causes, objMetaCauses...)

pkg/cloudproduct/gke/gke.go

@@ -112,8 +112,8 @@ func (*gkeAutopilot) NewPortAllocator(minPort, maxPort int32,
 
 func (*gkeAutopilot) WaitOnFreePorts() bool { return true }
 
-func (*gkeAutopilot) ValidateGameServerSpec(gss *agonesv1.GameServerSpec) []metav1.StatusCause {
-	var causes []metav1.StatusCause
+func (g *gkeAutopilot) ValidateGameServerSpec(gss *agonesv1.GameServerSpec) []metav1.StatusCause {
+	causes := g.ValidateScheduling(gss.Scheduling)
 	for _, p := range gss.Ports {
 		if p.PortPolicy != agonesv1.Dynamic {
 			causes = append(causes, metav1.StatusCause{
@@ -123,13 +123,6 @@ func (*gkeAutopilot) ValidateGameServerSpec(gss *agonesv1.GameServerSpec) []meta
 			})
 		}
 	}
-	if gss.Scheduling != apis.Packed {
-		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseTypeFieldValueInvalid,
-			Field:   "scheduling",
-			Message: errSchedulingMustBePacked,
-		})
-	}
 	// See SetEviction comment below for why we block EvictionSafeOnUpgrade.
 	if gss.Eviction.Safe == agonesv1.EvictionSafeOnUpgrade {
 		causes = append(causes, metav1.StatusCause{
@@ -141,6 +134,17 @@ func (*gkeAutopilot) ValidateGameServerSpec(gss *agonesv1.GameServerSpec) []meta
 	return causes
 }
 
+func (*gkeAutopilot) ValidateScheduling(ss apis.SchedulingStrategy) []metav1.StatusCause {
+	if ss != apis.Packed {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Field:   "scheduling",
+			Message: errSchedulingMustBePacked,
+		}}
+	}
+	return nil
+}
+
 func (*gkeAutopilot) MutateGameServerPodSpec(gss *agonesv1.GameServerSpec, podSpec *corev1.PodSpec) error {
 	podSpecSeccompUnconfined(podSpec)
 	return nil

pkg/cloudproduct/gke/gke_test.go

@@ -133,18 +133,18 @@ func TestValidateGameServer(t *testing.T) {
 			want: []metav1.StatusCause{
 				{
 					Type:    "FieldValueInvalid",
-					Message: "portPolicy must be Dynamic on GKE Autopilot",
-					Field:   "bad-udp.portPolicy",
+					Message: "scheduling strategy must be Packed on GKE Autopilot",
+					Field:   "scheduling",
 				},
 				{
 					Type:    "FieldValueInvalid",
 					Message: "portPolicy must be Dynamic on GKE Autopilot",
-					Field:   "another-bad-udp.portPolicy",
+					Field:   "bad-udp.portPolicy",
 				},
 				{
 					Type:    "FieldValueInvalid",
-					Message: "scheduling strategy must be Packed on GKE Autopilot",
-					Field:   "scheduling",
+					Message: "portPolicy must be Dynamic on GKE Autopilot",
+					Field:   "another-bad-udp.portPolicy",
 				},
 				{
 					Type:    "FieldValueInvalid",

test/e2e/fleet_test.go

@@ -1165,6 +1165,7 @@ func TestUpdateFleetScheduling(t *testing.T) {
 
 	t.Run("Updating Spec.Scheduling on fleet should be updated in GameServer",
 		func(t *testing.T) {
+			framework.SkipOnCloudProduct(t, "gke-autopilot", "Autopilot does not support Distributed scheduling")
 			client := framework.AgonesClient.AgonesV1()
 
 			flt := defaultFleet(framework.Namespace)

test/e2e/gameserverallocation_test.go

@@ -51,6 +51,11 @@ func TestCreateFleetAndGameServerAllocate(t *testing.T) {
 			fleet := defaultFleet(framework.Namespace)
 			fleet.Spec.Scheduling = strategy
 			flt, err := fleets.Create(ctx, fleet, metav1.CreateOptions{})
+			if strategy != apis.Packed && framework.CloudProduct == "gke-autopilot" {
+				// test that Autopilot rejects anything but Packed and skip the rest of the test
+				assert.ErrorContains(t, err, "configuration is invalid")
+				return
+			}
 			if assert.NoError(t, err) {
 				defer fleets.Delete(ctx, flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
 			}
@@ -268,6 +273,9 @@ func TestMultiClusterAllocationOnLocalCluster(t *testing.T) {
 	for _, strategy := range fixtures {
 		strategy := strategy
 		t.Run(string(strategy), func(t *testing.T) {
+			if strategy == apis.Distributed {
+				framework.SkipOnCloudProduct(t, "gke-autopilot", "Autopilot does not support Distributed scheduling")
+			}
 			t.Parallel()
 			ctx := context.Background()
 
@@ -405,6 +413,9 @@ func TestCreateFullFleetAndCantGameServerAllocate(t *testing.T) {
 		strategy := strategy
 
 		t.Run(string(strategy), func(t *testing.T) {
+			if strategy == apis.Distributed {
+				framework.SkipOnCloudProduct(t, "gke-autopilot", "Autopilot does not support Distributed scheduling")
+			}
 			t.Parallel()
 			ctx := context.Background()