Implement Alpha field best practices (#1348)

Best practices outlined in https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#adding-unstable-features-to-stable-versions

Closes #1347

pkg/apis/agones/v1/gameserver.go

@@ -151,13 +151,9 @@ type GameServerSpec struct {
 	SdkServer SdkServer `json:"sdkServer,omitempty"`
 	// Template describes the Pod that will be created for the GameServer
 	Template corev1.PodTemplateSpec `json:"template"`
-	// Alpha describes the alpha properties for the GameServer.
-	Alpha AlphaSpec `json:"alpha,omitempty"`
-}
-
-// AlphaSpec contains the alpha properties of the GameServer.
-type AlphaSpec struct {
-	Players PlayersSpec `json:"players"`
+	// (Alpha, PlayerTracking feature flag) Players provides the configuration for player tracking features.
+	// +optional
+	Players *PlayersSpec `json:"players,omitempty"`
 }
 
 // PlayersSpec tracks the initial player capacity, and what webhooks to send events to when there are
@@ -225,7 +221,9 @@ type GameServerStatus struct {
 	Address       string                 `json:"address"`
 	NodeName      string                 `json:"nodeName"`
 	ReservedUntil *metav1.Time           `json:"reservedUntil"`
-	Alpha         AlphaStatus            `json:"alpha"`
+	// (Alpha, PlayerTracking feature flag)
+	// +optional
+	Players *PlayerStatus `json:"players"`
 }
 
 // GameServerStatusPort shows the port that was allocated to a
@@ -235,11 +233,6 @@ type GameServerStatusPort struct {
 	Port int32  `json:"port"`
 }
 
-// AlphaStatus is the alpha status values for a GameServer
-type AlphaStatus struct {
-	Players PlayerStatus `json:"players"`
-}
-
 // PlayerStatus stores the current player capacity values
 type PlayerStatus struct {
 	Count    int64 `json:"count"`
@@ -315,7 +308,12 @@ func (gs *GameServer) applyStatusDefaults() {
 	}
 
 	if runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
-		gs.Status.Alpha.Players.Capacity = gs.Spec.Alpha.Players.InitialCapacity
+		if gs.Spec.Players != nil {
+			if gs.Status.Players == nil {
+				gs.Status.Players = &PlayerStatus{}
+			}
+			gs.Status.Players.Capacity = gs.Spec.Players.InitialCapacity
+		}
 	}
 }
 
@@ -345,6 +343,17 @@ func (gss *GameServerSpec) applySchedulingDefaults() {
 // the returned array
 func (gss *GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, bool) {
 	var causes []metav1.StatusCause
+
+	if !runtime.FeatureEnabled(runtime.FeaturePlayerTracking) {
+		if gss.Players != nil {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueNotSupported,
+				Field:   "players",
+				Message: fmt.Sprintf("Value cannot be set unless feature flag %s is enabled,", runtime.FeaturePlayerTracking),
+			})
+		}
+	}
+
 	if devAddress != "" {
 		// verify that the value is a valid IP address.
 		if net.ParseIP(devAddress) == nil {

pkg/apis/agones/v1/gameserver_test.go

@@ -62,14 +62,16 @@ func TestGameServerFindGameServerContainer(t *testing.T) {
 func TestGameServerApplyDefaults(t *testing.T) {
 	t.Parallel()
 
+	ten := int64(10)
+
 	type expected struct {
 		protocol            corev1.Protocol
 		state               GameServerState
 		policy              PortPolicy
 		health              Health
 		scheduling          apis.SchedulingStrategy
 		sdkServer           SdkServer
-		alphaPlayerCapacity int64
+		alphaPlayerCapacity *int64
 	}
 	data := map[string]struct {
 		gameServer   GameServer
@@ -78,11 +80,11 @@ func TestGameServerApplyDefaults(t *testing.T) {
 		expected     expected
 	}{
 		"set basic defaults on a very simple gameserver": {
-			featureFlags: runtime.FeaturePlayerTracking + "=true",
+			featureFlags: string(runtime.FeaturePlayerTracking) + "=true",
 			gameServer: GameServer{
 				Spec: GameServerSpec{
-					Alpha: AlphaSpec{Players: PlayersSpec{InitialCapacity: 10}},
-					Ports: []GameServerPort{{ContainerPort: 999}},
+					Players: &PlayersSpec{InitialCapacity: 10},
+					Ports:   []GameServerPort{{ContainerPort: 999}},
 					Template: corev1.PodTemplateSpec{
 						Spec: corev1.PodSpec{Containers: []corev1.Container{
 							{Name: "testing", Image: "testing/image"},
@@ -105,7 +107,7 @@ func TestGameServerApplyDefaults(t *testing.T) {
 					GRPCPort: 9357,
 					HTTPPort: 9358,
 				},
-				alphaPlayerCapacity: 10,
+				alphaPlayerCapacity: &ten,
 			},
 		},
 		"defaults on passthrough": {
@@ -186,7 +188,6 @@ func TestGameServerApplyDefaults(t *testing.T) {
 			gameServer: GameServer{
 				Spec: GameServerSpec{
 					Ports: []GameServerPort{{PortPolicy: Static}},
-					Alpha: AlphaSpec{Players: PlayersSpec{InitialCapacity: 10}},
 					Template: corev1.PodTemplateSpec{
 						Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "testing", Image: "testing/image"}}}}},
 			},
@@ -346,7 +347,12 @@ func TestGameServerApplyDefaults(t *testing.T) {
 			assert.Equal(t, test.expected.scheduling, test.gameServer.Spec.Scheduling)
 			assert.Equal(t, test.expected.health, test.gameServer.Spec.Health)
 			assert.Equal(t, test.expected.sdkServer, test.gameServer.Spec.SdkServer)
-			assert.Equal(t, test.expected.alphaPlayerCapacity, test.gameServer.Status.Alpha.Players.Capacity)
+			if test.expected.alphaPlayerCapacity != nil {
+				assert.Equal(t, *test.expected.alphaPlayerCapacity, test.gameServer.Status.Players.Capacity)
+			} else {
+				assert.Nil(t, test.gameServer.Spec.Players)
+				assert.Nil(t, test.gameServer.Status.Players)
+			}
 		})
 	}
 }
@@ -491,6 +497,36 @@ func TestGameServerValidate(t *testing.T) {
 	assert.Len(t, causes, 2)
 	assert.Contains(t, fields, "one.containerPort")
 	assert.Contains(t, fields, "two.hostPort")
+
+	t.Run("validation of "+string(runtime.FeaturePlayerTracking), func(t *testing.T) {
+		gs := GameServer{
+			Spec: GameServerSpec{
+				Players: &PlayersSpec{InitialCapacity: 10},
+				Template: corev1.PodTemplateSpec{
+					Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "testing", Image: "testing/image"}}}}},
+		}
+		gs.ApplyDefaults()
+		causes, ok := gs.Validate()
+		for _, f := range causes {
+			fields = append(fields, f.Field)
+		}
+		assert.False(t, ok)
+		assert.Len(t, causes, 1)
+		assert.Contains(t, fields, "players")
+
+		err := runtime.ParseFeatures(string(runtime.FeaturePlayerTracking) + "=true")
+		assert.NoError(t, err)
+		gs = GameServer{
+			Spec: GameServerSpec{
+				Players: &PlayersSpec{InitialCapacity: 10},
+				Template: corev1.PodTemplateSpec{
+					Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "testing", Image: "testing/image"}}}}},
+		}
+		gs.ApplyDefaults()
+		causes, ok = gs.Validate()
+		assert.True(t, ok)
+		assert.Empty(t, causes)
+	})
 }
 
 func TestGameServerPod(t *testing.T) {

pkg/util/runtime/features.go

@@ -31,7 +31,7 @@ const (
 	FeatureExample Feature = "Example"
 
 	// FeaturePlayerTracking is a feature flag to enable/disable player tracking features.
-	FeaturePlayerTracking = "PlayerTracking"
+	FeaturePlayerTracking Feature = "PlayerTracking"
 )
 
 var (