Skip to content

Commit

Permalink
Make Alpha & Beta CRD declaration consistent. (#2982)
Browse files Browse the repository at this point in the history 
We realised that with the CRD schema application being hidden behind a
feature flag in the helm CRD yaml, then if the feature flag was turned
off, an end user wouldn't get a validation error since the values passed
in would be pruned by the K8s control plane.

To make thins consistent across Alpha CRD values:

* Alpha & Beta schemas are always available within the CRD definitions.
* Validation checks are in place for the feature flag (no change needed)
* Alpha & Beta CRD values are pointers, so they can be nullable (this
required a change in Eviction handling and ApiHooks interface)
* Some general cleanup of comments and descriptions in the schema.
  • Loading branch information
@markmandel
markmandel authored Feb 23, 2023
1 parent 08c1eaa commit ba0ddbb
Show file tree
Hide file tree
Showing 15 changed files with 236 additions and 59 deletions.
14 changes: 7 additions & 7 deletions install/helm/agones/templates/crds/_gameserverspecschema.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ properties:
type: integer
title: The initial player capacity of this Game Server
minimum: 0
{{- if .featureCountsAndLists }}
counters:
type: object
title: Map of player, room, session, etc. counters
Expand All @@ -162,11 +161,13 @@ properties:
additionalProperties:
type: object
properties:
count: # initial count
count:
title: Initial count value
type: integer
default: 0
minimum: 0
capacity: # max capacity of the counter
capacity:
title: Max capacity of the counter
type: integer
minimum: 0
lists:
Expand All @@ -177,18 +178,18 @@ properties:
additionalProperties:
type: object
properties:
capacity: # max capacity of the array (can be less than or equal to value of maxItems)
capacity:
type: integer
title: Max capacity of the array (can be less than or equal to value of maxItems)
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
{{- end }}
{{- if .featureSafeToEvict }}
eviction:
type: object
title: Eviction tolerance of the game server
Expand All @@ -211,4 +212,3 @@ properties:
minimum: 1
maximum: 1
{{- end }}
{{- end }}
13 changes: 6 additions & 7 deletions install/helm/agones/templates/crds/_gameserverstatus.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ status:
nullable: true
items:
type: string
{{- if .featureCountsAndLists }}
counters:
type: object
title: Map of player, room, session, etc. counters
Expand All @@ -74,11 +73,12 @@ status:
additionalProperties:
type: object
properties:
count: # initial count
count:
title: The current count
type: integer
default: 0
minimum: 0
capacity: # max capacity of the counter
capacity:
type: integer
minimum: 0
lists:
Expand All @@ -89,18 +89,18 @@ status:
additionalProperties:
type: object
properties:
capacity: # max capacity of the array (can be less than or equal to value of values.maxItems)
capacity:
title: Max capacity of the array (can be less than or equal to value of values.maxItems)
type: integer
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: Set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
{{- end }}
{{- if .featureSafeToEvict }}
eviction:
type: object
properties:
Expand All @@ -117,4 +117,3 @@ status:
minimum: 1
maximum: 1
{{- end}}
{{- end}}
2 changes: 1 addition & 1 deletion install/helm/agones/templates/crds/gameserver.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ spec:
type: date
schema:
openAPIV3Schema:
{{- $data := dict "podPreserveUnknownFields" .Values.gameservers.podPreserveUnknownFields "featureSafeToEvict" $featureGates.SafeToEvict "featureCountsAndLists" $featureGates.CountsAndLists }}
{{- $data := dict "podPreserveUnknownFields" .Values.gameservers.podPreserveUnknownFields }}
{{- include "gameserver.schema" $data | indent 9 }}{{- /* include the schema then status, as it's easier to align */ -}}
{{- include "gameserver.status" $data | indent 11 }}
{{- if $featureGates.SafeToEvict }}
Expand Down
147 changes: 147 additions & 0 deletions install/yaml/install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5099,6 +5099,43 @@ spec:
type: integer
title: The initial player capacity of this Game Server
minimum: 0
counters:
type: object
title: Map of player, room, session, etc. counters
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
count:
title: Initial count value
type: integer
default: 0
minimum: 0
capacity:
title: Max capacity of the counter
type: integer
minimum: 0
lists:
type: object
title: Map of player, room, session, etc. lists
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
capacity:
type: integer
title: Max capacity of the array (can be less than or equal to value of maxItems)
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
eviction:
type: object
title: Eviction tolerance of the game server
Expand Down Expand Up @@ -10083,6 +10120,43 @@ spec:
type: integer
title: The initial player capacity of this Game Server
minimum: 0
counters:
type: object
title: Map of player, room, session, etc. counters
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
count:
title: Initial count value
type: integer
default: 0
minimum: 0
capacity:
title: Max capacity of the counter
type: integer
minimum: 0
lists:
type: object
title: Map of player, room, session, etc. lists
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
capacity:
type: integer
title: Max capacity of the array (can be less than or equal to value of maxItems)
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
eviction:
type: object
title: Eviction tolerance of the game server
Expand Down Expand Up @@ -10155,6 +10229,42 @@ spec:
nullable: true
items:
type: string
counters:
type: object
title: Map of player, room, session, etc. counters
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
count:
title: The current count
type: integer
default: 0
minimum: 0
capacity:
type: integer
minimum: 0
lists:
type: object
title: Map of player, room, session, etc. lists
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
capacity:
title: Max capacity of the array (can be less than or equal to value of values.maxItems)
type: integer
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: Set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
eviction:
type: object
properties:
Expand Down Expand Up @@ -15197,6 +15307,43 @@ spec:
type: integer
title: The initial player capacity of this Game Server
minimum: 0
counters:
type: object
title: Map of player, room, session, etc. counters
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
count:
title: Initial count value
type: integer
default: 0
minimum: 0
capacity:
title: Max capacity of the counter
type: integer
minimum: 0
lists:
type: object
title: Map of player, room, session, etc. lists
nullable: true
maxProperties: 1000
additionalProperties:
type: object
properties:
capacity:
type: integer
title: Max capacity of the array (can be less than or equal to value of maxItems)
minimum: 0
maximum: 1000 # must be equal to values.maxItems
values:
title: set of all the items in the list
type: array
x-kubernetes-list-type: set # Requires items in the array to be unique
maxItems: 1000 # max possible size of the value array (cannot be updated)
items: # name of the item (player1, session1, room1, etc.)
type: string
eviction:
type: object
title: Eviction tolerance of the game server
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/agones/v1/apihooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ type APIHooks interface {
MutateGameServerPodSpec(*GameServerSpec, *corev1.PodSpec) error

// SetEviction is called by gs.Pod to enforce GameServer.Status.Eviction.
SetEviction(EvictionSafe, *corev1.Pod) error
SetEviction(*Eviction, *corev1.Pod) error
}
6 changes: 3 additions & 3 deletions pkg/apis/agones/v1/apihooksfake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type fakeAPIHooks struct {
StubValidateGameServerSpec func(*GameServerSpec) []metav1.StatusCause
StubValidateScheduling func(apis.SchedulingStrategy) []metav1.StatusCause
StubMutateGameServerPodSpec func(*GameServerSpec, *corev1.PodSpec) error
StubSetEviction func(EvictionSafe, *corev1.Pod) error
StubSetEviction func(*Eviction, *corev1.Pod) error
}

var _ APIHooks = fakeAPIHooks{}
Expand Down Expand Up @@ -56,9 +56,9 @@ func (f fakeAPIHooks) MutateGameServerPodSpec(gss *GameServerSpec, podSpec *core
}

// SetEviction is called by gs.Pod to enforce GameServer.Status.Eviction.
func (f fakeAPIHooks) SetEviction(safe EvictionSafe, pod *corev1.Pod) error {
func (f fakeAPIHooks) SetEviction(eviction *Eviction, pod *corev1.Pod) error {
if f.StubSetEviction != nil {
return f.StubSetEviction(safe, pod)
return f.StubSetEviction(eviction, pod)
}
return nil
}
17 changes: 12 additions & 5 deletions pkg/apis/agones/v1/gameserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ type GameServerSpec struct {
Lists map[string]ListSpec `json:"lists,omitempty"`
// (Alpha, SafeToEvict feature flag) Eviction specifies the eviction tolerance of the GameServer. Defaults to "Never".
// +optional
Eviction Eviction `json:"eviction,omitempty"`
Eviction *Eviction `json:"eviction,omitempty"`
// immutableReplicas is present in gameservers.agones.dev but omitted here (it's always 1).
}

Expand Down Expand Up @@ -293,7 +293,8 @@ type GameServerStatus struct {
// +optional
Players *PlayerStatus `json:"players"`
// (Alpha, SafeToEvict feature flag) Eviction specifies the eviction tolerance of the GameServer.
Eviction Eviction `json:"eviction,omitempty"`
// +optional
Eviction *Eviction `json:"eviction,omitempty"`
// immutableReplicas is present in gameservers.agones.dev but omitted here (it's always 1).
}

Expand Down Expand Up @@ -422,6 +423,9 @@ func (gss *GameServerSpec) applyEvictionDefaults() {
if !runtime.FeatureEnabled(runtime.FeatureSafeToEvict) {
return
}
if gss.Eviction == nil {
gss.Eviction = &Eviction{}
}
if gss.Eviction.Safe == "" {
gss.Eviction.Safe = EvictionSafeNever
}
Expand All @@ -431,8 +435,11 @@ func (gs *GameServer) applyEvictionStatus() {
if !runtime.FeatureEnabled(runtime.FeatureSafeToEvict) {
return
}
gs.Status.Eviction = gs.Spec.Eviction
gs.Status.Eviction = gs.Spec.Eviction.DeepCopy()
if gs.Spec.Template.ObjectMeta.Annotations[PodSafeToEvictAnnotation] == "true" {
if gs.Status.Eviction == nil {
gs.Status.Eviction = &Eviction{}
}
gs.Status.Eviction.Safe = EvictionSafeAlways
}
}
Expand Down Expand Up @@ -469,7 +476,7 @@ func (gss *GameServerSpec) validateFeatureGates() []metav1.StatusCause {
}

if !runtime.FeatureEnabled(runtime.FeatureSafeToEvict) {
if gss.Eviction.Safe != "" {
if gss.Eviction != nil {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueNotSupported,
Field: "eviction.safe",
Expand Down Expand Up @@ -747,7 +754,7 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor
if err := apiHooks.MutateGameServerPodSpec(&gs.Spec, &pod.Spec); err != nil {
return nil, err
}
if err := apiHooks.SetEviction(gs.Status.Eviction.Safe, pod); err != nil {
if err := apiHooks.SetEviction(gs.Status.Eviction, pod); err != nil {
return nil, err
}

Expand Down
Loading

0 comments on commit ba0ddbb

Please sign in to comment.